Instrument branches

[DanRyanIrish]

As this package is new, some instrument subpackages may need rapid development and slightly looser standards on individual PRs. Therefore, in order that multiple haphazard instrument-specific development efforts do not turn the package into an unmanageable mess, development should be modularized between instruments.

Proposed Procedure: there should be instrument branches on which people can collaborate and contribute code for a specific instrument, e.g. iris. The contribution procedure would then be the following for different scenarios:

Implementation of a new feature or roadmap for a certain instrument

• New instrument-specific feature developed.
• Developer opens PR to instrument branch.
• PR is iterated on and merged.
• Subsequent PRs opened and merged if feature or roadmap requires several.
• Once feature(s) implemented, passing tests, and ready for release, open PR to master branch. This reduces the amount of buggy or incomplete code making into master. This minimizes the chance of the release of the whole package being held up by one instrument.

Feature required for whole package, e.g. retemplating

• Open PR directly to master

Bugfix on instrument branch not yet in master

• Open PR to instrument branch

Bugfix on master for specific instrument

• Open PR directly to master.
• Update instrument branch by pulling master and resolving any conflicts.
  If bug-fix, PR can be cherry-picked/"backported" to master.

Need for change outside of instrument sub-packages discovered as part of instrument specific development.

• Separate package and instrument-specific features in different PRs where the package PR is to master and the instrument PR is to the instrument branch.
• If necessary, merge developer master branch into developer instrument branch to continue work.
• Merge package PR first, and update instrument branch before merging new feature to instrument branch.

[DanRyanIrish]

@hayesla any thoughts?

[wtbarnes]

I can see the logic behind this, but doesn't this just offload the problem to the branches rather than the individual PRs? By creating separate branches, you're allowing the individual instrument efforts to potentially diverge quite a bit and the longer you hold off on merging them into master (or rebasing them), the worse that situation could become. I see your point about being able to iterate on a branch before putting it into master, but why not just do this iteration on the PR? Why merge it at all if the tests aren't passing? Alternatively, we could be a bit more lax about what gets merged and when.

I think this package should be moving (and releasing) fast enough that we shouldn't need to add this additional complication (this also places a larger burden on the maintainers and the instrument teams who may not be as familiar with a git-based workflow). This is part of the point of moving it out of core.

I'm not dead set against this idea, but I think we should be cautious about introducing an extra complication into our dev workflow. It is complicated enough already! 😅

[DanRyanIrish]

I can see the logic behind this, but doesn't this just offload the problem to the branches rather than the individual PRs?

I think this does add extra benefit. With this package there are possibly going to be many parallel development efforts by potentially quite different dev teams. They each may work at their own pace. In the early stages of development, they may want to merge PRs onto a common branch that aren't complete works into order to collaborate in getting the subpackage up to a working standard faster or more easily. Otherwise, developers may be having to pull random assortments of each other's branches to stay up to date. The instrument branch centralises that location and cuts down on confusion and divergence. The instrument branch also makes sure that only when the work is in a releasable state (or quite close to it) does the code make it into master. This means master is more likely to always be in (or close to) a release branch. This can help prevent the release of team's work being delayed because other teams have not got all their code in master to a releasable state.

By creating separate branches, you're allowing the individual instrument efforts to potentially diverge quite a bit and the longer you hold off on merging them into master (or rebasing them), the worse that situation could become.

Not if you regularly pull both master and the instrument branch. This is already required in how we work except that there is only one branch to update from, i.e. master. Adding a 2nd isn't a big overhead and reduces divergence, especially since most of the development between subpackages should be independent.

I see your point about being able to iterate on a branch before putting it into master, but why not just do this iteration on the PR?

As said above, multiple people want to help develop the subpackage to its first releasable state. This will likely be the case for what I have in mind for the IRIS subpackage. The instrument branch creates a sandbox in which subpackage teams can collaborate safely and quickly without polluting master for other subpackage teams. If this was done on a single PR, it would lead to multiple PRs to that PR and would get more messy and harder to keep track of.

Why merge it at all if the tests aren't passing? Alternatively, we could be a bit more lax about what gets merged and when.

In early stages of development there may not always be comprehensive tests to catch all bugs. In my experience the reality is that testing often does not catch up with the code until the subpackage is a little more mature. This is often because there is evolution of the package structure, API, and scope in the early stages and in reality people are inhibited from developing rigorous tests until there has been convergence on this.

I think this package should be moving (and releasing) fast enough that we shouldn't need to add this additional complication (this also places a larger burden on the maintainers and the instrument teams who may not be as familiar with a git-based workflow). This is part of the point of moving it out of core.

Unless the development efforts are modularized, this may not be true because without the instrument branches to act as staging areas, one instrument subpackage could hold up the whole package because they have introduced bugs or left features half finished on master. This would actually slow down the iteration of the overall package.

I'm not dead set against this idea, but I think we should be cautious about introducing an extra complication into our dev workflow. It is complicated enough already! 😅

This model is not that dissimilar to how some in industry work. A dev team has a common branch that they work on. When they are ready for release, it gets pushed up to the testing branch where a different team performs rigorous testing. When it passes their process it gets pushed to a release branch. Depending on how you want to think of it, in this model, master becomes the testing/release branch, while the instrument branches are the dev team's branch.

I see the benefits of this system mainly in the early stages of a subpackage's development where, as with sunpy core, it may be a little more like the "wild west" when it comes to merging PRs. Once v0.1 of sunkit-instruments has been released, we could re-assess whether this system still adds benefits.