[Practice] Reverting changes

[peternied]

Description

The maintainers had a discussion on how reverts have been done in the
past and how we would like to ensure the codebase is always ready for
contribution. The theme of that discussion was prioritizing
stable and available codebase - documenting that in this change.

Check List

• New functionality includes testing
• New functionality has been documented
• Commits are signed per the DCO using --signoff

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.

[cliu123]

Are we make reverting commits a part of regular basis practices? Based on my understanding of OpenSearch best practices, reverting should be minimized. But this is a good topic to discuss on organization level. @peternied Thanks for bringing this up! Thoughts? @opensearch-project/opensearch-core (I don't see a tag for all OpenSearch maintainers.)

[peternied]

Are we make reverting commits a part of regular basis practices?

@cliu123 Could you include the language that you'd like to see changed or your recommendation you have?

[cliu123]

The language looks good to me. My question is on the process of reverting and how we can minimize revert.

[davidlago]

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.

[codecov-commenter]

Codecov Report

Merging #1754 (1ffc83e) into main (54a920b) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1754   +/-   ##
=========================================
  Coverage     60.42%   60.42%           
+ Complexity     3197     3196    -1     
=========================================
  Files           253      253           
  Lines         18093    18093           
  Branches       3245     3245           
=========================================
  Hits          10933    10933           
  Misses         5579     5579           
  Partials       1581     1581           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54a920b...1ffc83e. Read the comment docs.

[peternied]

But this is a good topic to discuss on organization level.

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

[andrross]

Based on my understanding of OpenSearch best practices, reverting should be minimized.

We should certainly strive towards not introducing commits that cause impact, and we should continuously review and update processes that give us confidence that commits are good before merging them. However, no process is perfect and we're all human, so mistakes will be made. If a commit is merged and causes problems, then the revert-first-then-investigate approach is very reasonable because it is the fastest way to ensure that other folks do not remain blocked by the problem. Reverts are a fast and safe way to get a code base back to a known-good state (and are easy to undo themselves) so I don't see any particular reason to be wary of them. I'd love to hear if anyone has a differing opinion.

[cliu123]

Based on my understanding of OpenSearch best practices, reverting should be minimized.

We should certainly strive towards not introducing commits that cause impact, and we should continuously review and update processes that give us confidence that commits are good before merging them. However, no process is perfect and we're all human, so mistakes will be made. If a commit is merged and causes problems, then the revert-first-then-investigate approach is very reasonable because it is the fastest way to ensure that other folks do not remain blocked by the problem. Reverts are a fast and safe way to get a code base back to a known-good state (and are easy to undo themselves) so I don't see any particular reason to be wary of them. I'd love to hear if anyone has a differing opinion.

Totally agree. The PR itself brings up a good topic, so no wariness on the idea at all. My take is that the entire organization should have a consistent process/policy/documentation on the process of reverting commit.

[peternied]

@andrross @owaiskazi19 Thanks again from the thoughtful comments - consider bringing this topic to your own teams or adding practices like this in the .github repo.

[owaiskazi19]

@andrross @owaiskazi19 Thanks again from the thoughtful comments - consider bringing this topic to your own teams or adding practices like this in the .github repo.

Sure. Can you please create an issue on OpenSearch for the same? We can take it from there.

[peternied]

@owaiskazi19 I'll leave this to you and the OpenSearch core team to drive

[opensearch-trigger-bot]

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1754-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 837ca69d9bb91a63ef0d8cfc8297088d776ca19e
# Push it to GitHub
git push --set-upstream origin backport/backport-1754-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1754-to-2.x.

[opensearch-trigger-bot]

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.x 1.x
# Navigate to the new working tree
cd .worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-1754-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 837ca69d9bb91a63ef0d8cfc8297088d776ca19e
# Push it to GitHub
git push --set-upstream origin backport/backport-1754-to-1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-1754-to-1.x.

[opensearch-trigger-bot]

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-1754-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 837ca69d9bb91a63ef0d8cfc8297088d776ca19e
# Push it to GitHub
git push --set-upstream origin backport/backport-1754-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-1754-to-1.3.

[peternied]

FYI - Manually backport associated with #2843