Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PROPOSAL] Feature branch development #117

Closed
ashwin-pc opened this issue Dec 9, 2022 · 3 comments
Closed

[PROPOSAL] Feature branch development #117

ashwin-pc opened this issue Dec 9, 2022 · 3 comments

Comments

@ashwin-pc
Copy link
Member

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
Copy link
Member

dblock commented Dec 12, 2022

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
Copy link
Member Author

ashwin-pc commented Dec 15, 2022

@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
Copy link
Member

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 opensearch-project/opensearch-dashboards-functional-test#469

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants