search

opensearch-project / .github

Provides templates and resources for other OpenSearch project repositories.
Apache License 2.0
28 stars 71 forks source link

[PROPOSAL] Feature branch development #117

Closed ashwin-pc closed 1 year ago

ashwin-pc commented 1 year ago

What/Why

What are you proposing?

This is based on the original proposal in OpenSearch Dashboards. We use a hybrid of the feature branch development model and the Continuous integration development model [1]. We introduce 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 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.

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

What users have asked for this feature?

Many big feature projects for OpenSearch Dashboards have asked to use the feature branch model to collaborate on the main repo and asked for guidance on the correct practices to follow there. e.g. Alerting anywhere, Point in time, Multiple data sources etc. The observability team too has to recently used this model but has run into some of the issues outlined below when using it.

What problems are you trying to solve?

The feature branch model today is not well defined which allows for different style of this model being used to build features that cause issues down the road like

  1. A massive PR to main that is very hard to accurately review
  2. Code quality suffers because the intermediate PR's aren't reviewed carefully since many of them are work in progress, missteps are ignored as followup tasks that arent tracked.
  3. Breaking changes arent identified until the last minute due to newer changes in main

What is the developer experience going to be?

The initial attempt will be slightly slower since the initial bootstrapping effort for a feature may take longer to meet the bar for a valid feature branch PR, but subsequent changes should be quicker since we are sure that the changes introduced so far will not break the experience and that we dont have to work around them. It also makes asynchronous collaboration easier.

Are there any security considerations?

No

Are there any breaking changes to the API

no

What is the user experience going to be?

Not user facing

Are there breaking changes to the User Experience?

No

Why should it be built? Any reason not to?

It can slow down the pace of feature development since it has additional restrictions and does not allow for work in progress code. However this tradeoff is not as significant since the developer is always free to make such changes in their local fork and these limitations only apply to the feature branches in the original repo.

What will it take to execute?

Codifying this proposal in the repo and using it as guidance for other project repo's to follow

Any remaining open questions?

No

dblock commented 1 year ago

I would welcome a new section somewhere in the .md's of this project called "Feature Branches" that explains the roles of maintainers in those (eg. branch protection?).

Note that opensearch-project/OpenSearch extensively uses feature branches. Some like extensioins (@dbwiddis @saratvemulapalli) will have less stringent criteria for merging (opposite of what you're proposing), and they will extract stable code into brand new PRs to main.

ashwin-pc commented 1 year ago

@dblock I agree. If we have consensus on the proposal after hearing from the others (e.g. @saratvemulapalli and @dbwiddis) I can transform this into a readme file in this repo. I would love to know how those trade off work when they have to inevitably merge to main.

Also it might be worth calling out that point 2 can be overlooked when there are good reason to not have such stringent PR requirements. We are currently evaluating how stringent this needs to be in OSD using the PIT and AD Anywhere projects to validate this process. Would love to hear feedback from others about this.

joshuarrrr commented 1 year ago

One other consideration (for dashboards and dashboard plugins) is that it's helpful to also create a feature branch in the functional test repo - see https://github.com/opensearch-project/opensearch-dashboards-functional-test/issues/469