Aus development workflow proposal

I’ve run into a few hiccups in the Australian development pipeline, and have a suggestion for a change to improve the process.

The current process

We develop code in a branch, and then use the Open Food Network Deploy Buildkite pipeline to push it to staging. The branch is tested, and feedback goes on the related github issue (most commonly) or sometimes in a discourse thread.

This process has three problems: There’s no code review built in, so it’s fairly common for code that’s never been seen by another developer to go to production. Secondly, the home for testing feedback is not clearly defined, and it’s harder than it could be to assess the progress of features through the review/test/push process. Lastly, there are separate processes for handling internal work and pull requests submitted by other developers.

The suggested change

I suggest that Aus dev team developers open a pull request for each feature when it’s ready for testing, referencing the relevant GitHub issue that spawned the PR. From here, it’s easy for another developer to perform a code review, and all testing feedback can go in the pull request.

The current system is not optimally set up for this workflow. When an Aus dev team member pushes a branch and opens a PR for it, Travis performs a separate build of both the branch and the PR, locking up CI for longer than needed. I suggest we configure Travis to build only PRs and the master branch.

Within Buildkite, we could now deploy all branches using the Open Food Network Pull Requests pipeline, and we could drop the Open Food Network Deploy pipeline.

RFC: @oeoeaio @maikel @bingxie @danielle @Kirsten

Thank you for bringing this up. I think it would be good to have pull requests for everybody.

@oeoeaio started a while ago to push branches to his own fork of OFN. I’m doing that now as well. So we can all have our own Travis resources independently. The only drawback is that you need to push to your fork’s master branch from time to time. Otherwise Travis’ bundler cache is out of date and build times increase.

The new Open Food Network Pull Requests pipeline still needs Travis to build other branches than master though. If a branch is not merged before staging, it merges and pushes the results as merged-pull/xxxx where xxxx is the number of the pull request. Travis can then test it while the code is staged. A merge back into master happens only if the merged code passed on Travis as well. Unfortunately, there is no mechanism to clean up these branches once merged.

Github also added a new feature for pull request reviews. I have started using it and it’s pretty good. You can “Approve” or “Request changes”.

@maikel is the generally preferred approach now to create PRs from personal forks of OFN? Rather than pushing branches to openfoodnetwork?

Yes. That’s what we do here in Aus now. It mainly gives us more Travis time and it is more clear to whom branches belong.