bugzilla harmony saturday update

PSGI seems pretty stable, so now to make the rest actually work!

First I’m moving schema changes out of extensions. This doesn’t pass tests yet — and it is not complete. I’ll have to update templates as well.

Next, a lot of our UX code assumes a workflow that is different than the default values. Ideally we’d support changing the workflow, but for the moment
the default workflow provided should be the one the bmo one.

Finally, we’re still getting an error because the product security group doesn’t yet have a good default. My early attempt this is not entirely working…

From now, whatever the last thing I’ve been working on will be in this unstable branch, because in a week or two I hope to have master in a consistent, release-ready state.

I’ll spend a bit of time Sunday as well, and hope to have something runnable without having to run the scripts/generate_bmo_data.pl hack.

happy bmo push day!

release tag

the following changes have been pushed to bugzilla.mozilla.org:

  • [1437383] Create User.pm PhabBugz class for loading of a user object from phabricator
  • [1441329] Fix typos in the PhahBugz User.pm module
  • [1438206] Process SES email bounces properly
  • [1441475] BMO is vulnerable to reverse tabbnabbing
  • [1437384] phabbugz_feed.pl in PhabBugz extension should be extended to also query for new users in Phab
  • [1403344] Extract schema migration code from checksetup.pl and expose via docker container command
  • [1429621] Add Saved Searches to My Dashboard
  • [1433299] Link in summary is broken
  • [1384313] Can’t build the docs from within the vagrant box
  • [1441569] remove_idle_group_members.pl fails on vagrant box
  • [1440239] Assign a secure revision to the `secure-revision` group project
  • [1437646] Support Mozlog logs using Log::Log4perl
  • [1442099] Add memcached tracing to help debug weirdness in cloud env
  • [1442288] Bugzilla::Logging should log when a program is being run interactively
  • [1442520] move inbound_proxies to localconfig
  • [1402494] BMO Integration User is a full administrative user on Phabricator
  • [1443003] Port bug 1175211 to Harmony branch (Undefined subroutine &Bugzilla::CGI::SERVER_PUSH)
  • [1273381] Improve bugzilla object performance by using Class::XSAccessor for object accessors
  • [1419973] Modify product selector layout on Browse and Enter Bug pages
  • [1429344] Review requests in requests dropdown should link to MozReview or GitHub instead of Bugzilla details page
  • [1433573] Display the short URL link even for queries without any results
  • [1443049] is_interactive() must be declared before log4perl config is loaded
  • [1343248] Migrate secbugstats scripts to bmo production
  • [1441181] Implement new process model for running multiple email jobqueue daemons

discuss these changes on mozilla.tools.bmo.

Small Pull Requests, a week-ish later

I think this “chains of PRs” thing is working quite well. I’ve been playing around with how I name the branches, and thinking harder about automating it.

Of course it’s really ballooning the number unreviewed PRs I have.
I think my new name might be Dylan “Twenty Unreviewed Pull Requests” Hardison.

But some observations:

  1. Even with no automation, this isn’t much more work for me.
  2. Smaller commits lend themselves to drive-by reviews.
  3. I’ve also observed more mistakes being found.

So I’m going to continue to work this way for all larger tasks.

I wonder how many PRs it will be to implement oauth2?

An experiment for making big changes with smaller commits on GitHub.

It is safe to say that smaller commits is a perennial topic at Mozilla.

On the reviewer side, I definitely prefer smaller commits.
I find it’s easier — and faster — to review them. Often not in the neighborhood of microcommits, but at least 300-400 line patches.

For the thing I work on we use GitHub, and PRs seem to work best on small things. When tasked with a large task, it seems that many engineers will break it up into suitably small PRs and things work well.

However sometimes tasks are more complicated than you’d imagine.

Bug 1437646 seemed simple — let’s just log JSON to standard out. But logging json from apache is hard (for a variety of reasons outside the scope of this post).

At first I was able to split this into two tasks:
1. add a log server that listens on localhost:5880 and serializes all lines sent to it to stdout.
2. (finally) implement Log::Log4perl

However Bugzilla is not simple; it does not yield easily to such changes.
We need a place to keep the log4perl config file(s), and the wrapper I wrote to start apache in docker isn’t smart enough to wait until our socket-logging-server is running. So what was planned as roughly two tasks resulted in two relatively large PRs.

So the problem is — how can I make this easier to review, using the tools available? In the case my own discomfort is an an acceptable trade-off if it means my code can be reviewed more easily.

Below is what I came up with — and it’s immediately useful though far from perfect.

  1. Work out a series of commits (microcommits or at least smaller ones)
  2. for each commit, create a branch. This means for 4 commits you have 4 branches. For the examples below we’ll use feature/1, feature/2, feature/3, and feature/4. Imagine that “feature” is a more descriptive word.
  3. For feature/1, make a PR that would merge it into feature/0-reviewed
  4. Make a branch that is identical to feature/1, called feature/1-reviewed
  5. For feature/2, make a PR that merge it into feature/1-reviewed
  6. Continue this until the end. Every feature/N will merge into feature/N-reviewed, and feature/N-reviewed will be equal to feature/(N -1).

As a result of doing this, there will be 4 PRs that each only show the differences of the commits in question. After they’re all reviewed, it will still be required to cherry-pick each of the reviewed commits into a final branch and make a PR out of that, but getting that (rather larger) PR approved
can be a less involved process than code review — perhaps even the responsibility of a bot?

I’ve got some shell scripts to accomplish this, so I’m going to try it a bit and see what the results are.

happy bmo push day!

release tag

the following changes have been pushed to bugzilla.mozilla.org:

  • [1433993] Outdated FreeOTP link in user preferences
  • [1433833] Add index to email_rates.message_ts
  • [1436301] Exempt bot accounts from idle group removal
  • [1430259] Update policy code in BMO PhabBugz extension to update custom policy if a private bugs groups have changed.
  • [1343248] Migrate secbugstats scripts to bmo production
  • [1434064] Refactor Project.pm to use Moo for better type checking
  • [1434438] Refactor Revision.pm to use Moo for cleaner type checking
  • [1424363] securemail help page recommends getting a certificate from StartCom
  • [1433400] Prevent cross-site image requests from leaking contents of certain fields due to regex search
  • [1435735] Add script to add email job to jobqueue
  • [1211377] needinfo from someone not in sec group shows warning message even when bug is being removed from sec group
  • [1432296] Prevent bugzilla static assets from being blocked by overly long request URIs

discuss these changes on mozilla.tools.bmo.