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.

Sign up for GitHub

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

Adds feature branch development guidelines #127

Merged
merged 2 commits into from
Feb 28, 2023
Merged

Adds feature branch development guidelines #127

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
696dc6e
Adds feature branch development guidelines
ashwin-pc Jan 12, 2023
7bb4ac1
addresses PR comments
ashwin-pc Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
- [First Things First](#first-things-first)
- [Ways to Contribute](#ways-to-contribute)
- [Bug Reports](#bug-reports)
- [Feature Requests](#feature-requests)
- [Feature Requests \& Proposals](#feature-requests--proposals)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOC doesn't match, update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change actually fixes the TOC for the Feature requests heading. The previous link does not actually work. It was my markdown linter that actually caught this error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked at @ashwin-pc's forks seem to work as expected. [link]

- [Documentation Changes](#documentation-changes)
- [Contributing Code](#contributing-code)
- [Developer Certificate of Origin](#developer-certificate-of-origin)
- [License Headers](#license-headers)
- [Java](#java)
- [Python, Ruby, Shell](#python-ruby-shell)
- [Review Process](#review-process)
- [Feature Branches](#feature-branches)

## Contributing to OpenSearch

Expand Down Expand Up @@ -133,3 +134,19 @@ During the PR process, expect that there will be some back-and-forth. Please try
If we accept the PR, a [maintainer](MAINTAINERS.md) will merge your change and usually take care of backporting it to appropriate branches ourselves.

If we reject the PR, we will close the pull request with a comment explaining why. This decision isn't always final: if you feel we have misunderstood your intended change or otherwise think that we should reconsider then please continue the conversation with a comment on the PR and we'll do our best to address any further points you raise.

## Feature Branches

For long running features that need collaboration between multiple members of the community, we use feature branches. We use a hybrid of the feature branch development model and the continuous integration development model. Maintainers of a repository can create a feature branch, a corresponding label associated with the feature and a meta issue associated with the feature. Each feature branch will have the following safeguards:

1. High frequency integration: PR's to `main` are raised not once the feature is complete, but whenever we reach the closest integration point during development. For example, once the plugin is bootstrapped, or a core feature is complete. We also frequently rebase changes from main back into the feature branch.
1. This makes the integration frequency much higher and allows us to catch integration issues much quicker.
2. 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`. CI should run on all PR's, no incomplete work should be merged, tests are necessary, etc.
1. This maintains the code quality going into each feature making the integration to main PR's much easier and quicker.
2. 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.

All the safeguard here are not rules but guidelines and should be adopted by each repository based on their specific requirements. This is to ensure that feature branch development is less likely to have code quality issues and massive merge to main PR's.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good.

Run a spell checker, e.g. corrsponding is mispelled.
Make sure all sentences have periods at the end, like the first bullet and 2.1.

Consider shortening the text and maybe getting rid of bullet points. What are the most important points? I think the fact that feature branches need to match the process on main, but then you make a lot of assumptions about that process itself. We're not that prescriptive or we don't need to repeat all the things we do on main, saying "Feature branch processes, including but not limited to working through pull requests, doing code reviews and ensuring that CI/CD passes, should be the same as on main."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am hesitant on being less prescriptive here since that has often led to more confusion than less. Especially as the number of repositories that we have grows, being more opinionated is quite useful since it removes ambiguity. That being said, I do call out that these are not rules, but rather guidelines, so repo owners can make judgement calls as they see fit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section looks fine to me, thanks for cleaning up the structure.


For contributors looking to add a new feature that would require the creation of a feature branch, the process begins by opening an issue in the repo with the feature proposal. Depending on the nature of the feature, the maintainers of the project can decide to create a feature branch and use this model.