Adds feature branch development guidelines #127

ashwin-pc · 2023-01-12T05:14:15Z

Signed-off-by: Ashwin Pc [email protected]

Description

Add feature branch development guidelines to the repo

Issues Resolved

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Ashwin Pc <[email protected]>

dblock

TOC and spelling is a must have, everything else up to you!

dblock · 2023-01-13T14:44:08Z

CONTRIBUTING.md

@@ -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)


TOC doesn't match, update.

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

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

CONTRIBUTING.md

dblock · 2023-01-13T14:53:29Z

CONTRIBUTING.md

+   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.


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."

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.

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

peternied

Love that you are documenting this process for how/when to make feature branches. Could you add some maintainers from Dashboards to take a look and provide feedback too?

FYI @opensearch-project/opensearch-dashboards-core

joshuarrrr

A couple of questions, but otherwise just some proofreading suggestions. I also agree with @dblock that we should streamline where possible to avoid being overly prescriptive, although I appreciate all of that detail and clarity at the repo level.

CONTRIBUTING.md

joshuarrrr · 2023-01-19T01:40:21Z

CONTRIBUTING.md

+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 and a corrsponding label 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. 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
+    1. This makes the integration frequency much higher allowing us to catch integration issues much quicker.


Suggested change

1. This makes the integration frequency much higher allowing us to catch integration issues much quicker.

1. This makes the integration frequency much higher and allows us to catch integration issues sooner.

CONTRIBUTING.md

joshuarrrr · 2023-01-19T01:44:43Z

CONTRIBUTING.md

+
+## 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 and a corrsponding label associated with the feature. Each feature branch will have the following safeguards:


Do we also want to mention that feature branches should have a meta issue associated with them? And would an example of a feature branch with all these features (meta issue, branch, label) be useful to link to?

Yeah i like the idea of a meta issue but do we have a good example yet to link here? Also since the feature branches will be deprecated once the feature is complete, the links will be outdated coon too.

joshuarrrr · 2023-01-19T01:46:47Z

CONTRIBUTING.md

+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. 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
+    1. This makes the integration frequency much higher allowing 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. i.e. CI is run on all PR's, no incomplete work should be merged, Tests are necessary for all PR's etc.


Suggested change

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.

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.

What about CHANGELOG enforcement? It seems sensible to make an exception for that (only editing the CHANGELOG as part of the PRs to main.

Given the open discussion around changelog and back-porting thats happening right now, i'd keep that out of this until we have clarity there

mingshl · 2023-01-24T00:14:22Z

CONTRIBUTING.md

+
+## 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 and a corrsponding label associated with the feature. Each feature branch will have the following safeguards:


Would you please point the direction(might be links) to guide the contributors towards finding what is the best way to ask the maintainers to create the feature branch? Should they create an issue and request maintainers? Please provide more details.

Good callout, i will add a sentence for contributors looking to introduce a new feature

peternied · 2023-01-24T19:57:41Z

@ashwin-pc Once the outstanding feedback has been addressed I will be happy to approval this change.

Signed-off-by: Ashwin Pc <[email protected]>

ashwin-pc · 2023-02-22T20:16:56Z

@peternied @dblock Ive addressed the comments here. Let me know if there are any other concerns you have about the PR.

peternied

Looks good to me @ashwin-pc.

Can we get another review?

peternied · 2023-02-27T20:29:02Z

CONTRIBUTING.md

@@ -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)


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

peternied · 2023-02-27T20:44:45Z

CONTRIBUTING.md

+   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.


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

dblock · 2023-02-28T18:10:14Z

Merged, thanks.

Adds feature branch development guidelines

696dc6e

Signed-off-by: Ashwin Pc <[email protected]>

ashwin-pc requested review from CEHENKLE, dblock, mch2, peternied and saratvemulapalli as code owners January 12, 2023 05:14

dblock requested changes Jan 13, 2023

View reviewed changes

peternied reviewed Jan 13, 2023

View reviewed changes

joshuarrrr reviewed Jan 19, 2023

View reviewed changes

mingshl reviewed Jan 24, 2023

View reviewed changes

addresses PR comments

7bb4ac1

Signed-off-by: Ashwin Pc <[email protected]>

peternied approved these changes Feb 27, 2023

View reviewed changes

dblock approved these changes Feb 28, 2023

View reviewed changes

dblock merged commit 33446db into opensearch-project:main Feb 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds feature branch development guidelines #127

Adds feature branch development guidelines #127

ashwin-pc commented Jan 12, 2023

dblock left a comment

dblock Jan 13, 2023

ashwin-pc Feb 22, 2023

peternied Feb 27, 2023

dblock Jan 13, 2023

ashwin-pc Feb 22, 2023

peternied Feb 27, 2023

peternied left a comment

joshuarrrr left a comment

joshuarrrr Jan 19, 2023

joshuarrrr Jan 19, 2023

ashwin-pc Feb 22, 2023

joshuarrrr Jan 19, 2023

ashwin-pc Feb 22, 2023

mingshl Jan 24, 2023

ashwin-pc Feb 22, 2023

peternied commented Jan 24, 2023

ashwin-pc commented Feb 22, 2023

peternied left a comment

peternied Feb 27, 2023

peternied Feb 27, 2023

dblock commented Feb 28, 2023

	1. This makes the integration frequency much higher allowing us to catch integration issues much quicker.
	1. This makes the integration frequency much higher and allows us to catch integration issues sooner.


		## 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 and a corrsponding label associated with the feature. Each feature branch will have the following safeguards:

	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.
	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.

Adds feature branch development guidelines #127

Adds feature branch development guidelines #127

Conversation

ashwin-pc commented Jan 12, 2023

Description

Issues Resolved

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternied left a comment

Choose a reason for hiding this comment

joshuarrrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternied commented Jan 24, 2023

ashwin-pc commented Feb 22, 2023

peternied left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Feb 28, 2023