[Proposal] Feature branch development

[ashwin-pc]

Feature branch development for long running features seems to be the preferred way for collaboration between different contributors. It also raises some concerns about code quality and PR size (when merging to main). While our initial attempt at this development model realized these concerns and halted further feature branch development, this proposal aims to address those concerns with an alternate workflow.

We can use a hybrid of the feature branch development model and the Continuous integration development model [1]. We reintroduce feature branch development with the following safeguards:

1. High frequency integration: We raise PR's to main not once the feature is complete, but whenever we reach the closest integration point during development. e.g. once the plugin is bootstrapped, or a core feature is complete. we also frequently rebase changes from main back into the feature branch (Thanks @kavilla for calling this out)
   a. This makes the integration frequency much higher allowing us to catch integration issues much quicker.
   b. This still lets collaboratively develop on a big feature that is not ready for main
2. Treat feature branch PR's the same as PR's to main. i.e. CI is run on all PR's, no incomplete work should be merged, Tests are necessary for all PR's etc.
   a. This maintains the code quality going into each feature making the integration to main PR's much easier and quicker
   b. More visibility during development since it gives reviewers the necessary time to review each PR.
3. Feature specific labels: This helps identify feature related issues and PR's. (Thanks @joshuarrrr)

This should address the main concerns raised during the initial few features that were developed using this flow. We can experiment with this model for a few features before committing to it too since development without it seems to be difficult at the moment.

[1]Ref: https://martinfowler.com/articles/branching-patterns.html

[kavilla]

I like it! For the feature branch we can also call out that merge PRs from main can be made into the feature branch to keep it up to date as possible. Also, enforces the tests as a requirement instead of tests as an after thought. Thanks @ashwin-pc for making this.

[joshuarrrr]

I like this approach - high-frequency integration solves much of the pain of one giant merge at the end. It should also work with our existing permissions model, with no changes necessary for contributors, correct?

[ashwin-pc]

Yep, this should work with our current permission model.

[joshuarrrr]

@ashwin-pc I have one additional suggestion - as part of creating new feature branches, can we also create a dedicated label? That makes it easy to filter on our out issues that are specific from the feature (otherwise they can sometimes swamp the repo). By default, maybe the label just matches the branch name?

[ashwin-pc]

Good call, Each of our features so far have had feature specific labels to help identify the feature specific issues and PR's. Will update the proposal.

[kavilla]

@ashwin-pc, @joshuarrrr,

One insight from the OpenSearch repo was that for the feature branch they are determined to get a main contributor to feature branches as a maintainer. Who has the insight of the application would enforce our standards so that way long running feature branches can be looked over with more trust. Food for thought.

[ashwin-pc]

Thats a nice idea but it may be hard to achieve, at least for the two feature branches we are aiming to introduce. Also the criteria that maintainers are still required to merge feature branch PR's should in theory give us the same level of trust without needing to have a single main contributor looking over the whole feature development in isolation.

[lezzago]

I think we can go ahead with this approach for the feature branch being created for #2880.

I do believe this will slow down development still as it would slow down development that is dependent on the new commit going into the feature branch. However, this should ideally hold a higher bar for when we merge to main and will be more confident when changes to get merged into main.

[ashwin-pc]

yeah, that's a tradeoff that we are experimenting with. The hope is that the initial tradeoff will result in much smoother development as the feature progresses and save time when the merges to main needs to occur. It will also help guide some of the foundational changes made for the feature w.r.t how it integrates with dashboards since that's where it most likely that less than ideal decisions might be made due to gaps in our documentation.