-
Notifications
You must be signed in to change notification settings - Fork 293
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
[Practice] Reverting changes #1754
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
- [OpenSearch Security Maintainers](#opensearch-security-maintainers) | ||
- [Maintainers](#maintainers) | ||
- [Updating Practices](#updating-practices) | ||
- [Practices](#practices) | ||
- [Reverting Commits](#reverting-commits) | ||
- [Performing Revert](#performing-revert) | ||
|
||
# OpenSearch Security Maintainers | ||
|
||
|
@@ -14,3 +17,15 @@ | |
|
||
### Updating Practices | ||
To ensure common practices as maintainers, all practices are expected to be documented here or enforced through github actions. There should be no expectations beyond what is documented in the repo [CONTRIBUTING.md](./CONTRIBUTING.md) and OpenSearch-Project [CONTRIBUTING.md](https://github.com/opensearch-project/.github/blob/main/CONTRIBUTING.md). To modify an existing processes or create a new one, make a pull request on this MAINTAINERS.md for review and merge it after all maintainers approve of it. | ||
|
||
# Practices | ||
|
||
## Reverting Commits | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@cliu123 Could you include the language that you'd like to see changed or your recommendation you have? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The language looks good to me. My question is on the process of reverting and how we can minimize revert. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the to-be-reverted change is blocking, failing builds or disrupting in other ways, and a fix forward is not quick, we should revert and work on a fix that reverts the revert plus any other changes needed to fix the initial issue. @cliu123 I believe that's what you are pointing at, as in reverting should be a last resort to unblock rather than the go-to strategy for fixing things, and I think that's exactly what that first paragraph on this PR conveys... but if it's not clear please feel free to suggest re-wording. |
||
There will be changes that destabilize or block contributions. The impact of these changes will be localized on the repository or even the entire OpenSearch project. We should bias towards keeping contributions unblocked by immediately reverting impacting changes, these reverts will be done by a maintainer. After the change has been reverted, an issue will be openned to re-merge the change and callout the elements of the contribution that need extra examination such as additional tests or even pull request workflows. | ||
|
||
Exceptional, instead of immediately reverting, if a contributor knows how and will resolve the issue in an hour we should fix-forward to reduce overhead. | ||
peternied marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Performing Revert | ||
Go to the pull request of the change that was an issue, there is a `Revert` button at the bottom. If there are no conflicts to resolve, this can be done immediately bypassing standard approval. | ||
|
||
Reverts can also be done via the command line using `git revert` and creating a new pull request. If done in this way they should have references to the pull request that was reverted. | ||
peternied marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cliu123 Different repositories inside OpenSearch-Project will have different requirements to avoid all of those situations, I am tailoring this document around our repo first.