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

Document what is a backport and how to cherry-pick commits #48

Open
viniarck opened this issue Feb 27, 2023 · 4 comments · May be fixed by #75
Open

Document what is a backport and how to cherry-pick commits #48

viniarck opened this issue Feb 27, 2023 · 4 comments · May be fixed by #75
Assignees
Labels
documentation Improvements or additions to documentation priority_low Low priority

Comments

@viniarck
Copy link
Member

This is for documenting what is a backport is and how to cherry-pick commits when an older version of a core projects or NApp needs to be patched and maintained

@viniarck viniarck added documentation Improvements or additions to documentation priority_low Low priority labels Feb 27, 2023
@viniarck
Copy link
Member Author

viniarck commented Dec 16, 2024

This can be done on https://github.com/kytos-ng/kytos-ng.github.io/blob/master/procedures/prs.md

By adding a new section, and then refining and better explaining the steps below:

Team, we've should've had better documented how to backport, we'll do it in the future, but in the meantime, here's a recap how to do it:

  • Find the latest version that you're going to backport, let's say it's 20XX.2.0, then you checkout that tag 20XX.2.0.
  • You create and push a new base branch incrementing the patch version base/20XX.2.1 (you need to be a core dev or maintainer to push a branch upstream, if you aren't ask for help).
  • You create another branch release/20XX.2.1 from base/20XX.2.1 and then when opening a PR you target base/20XX.2.1, so the release/20XX.2.1 branch will be your work branch and when gets merged will get deleted and ultimately the base/20XX.2.1 will be the branch that you should also use to create the release when tagging upstream.
  • Once your PR gets approved, merge into the base branch and tag upstream, the tag should target the base/20XX.2.1 in this example.
  • Finally, to double check you tagged correctly go to GitHub release pages, open the release again and click in the commit, it should display like a PR diff of what was included. Here's an example of an old release kytos-ng/kytos@d187380

@italovalcy
Copy link
Contributor

@viniarck very nice the documentation described on the steps above! If you allow me to send half-bit of a contribution:

  • I would suggest calling the work branch on the steps above with a different name just to avoid misunderstanding. I know this is just an example, but calling it "release/xyz" could lead to a confusion about the purpose of that work branch. One suggestion is to provide an example with something like "backport/issue_XYZ" where XYZ refers to the issue number that has to be created to describe the backport process, document priorities, assignees, etc.

  • We should state on the documentation that it is allowed to have multiple PRs targetting the same backport number, although it is recommended not to do so

  • We should also state on the documentation that reviewers should make sure end-to-end tests AND unit tests will be executed on source branch of the PR before approving it (if I'm not wrong, the CI/CD won't run automatically for non-master targeting branches, right?)

  • We should add some steps to notify slack channel about this new release

  • Maybe it would be nice to also have some text explaining the overall idea of creating the back ports, conditions where it applies and things to avoid (like, back ports should be created when critical bugs gets fixed or when the core development team agrees this is a critical change that should be backport to avoid issues or to implement key features for production scenarios).

@viniarck
Copy link
Member Author

I know this is just an example, but calling it "release/xyz" could lead to a confusion about the purpose of that work branch. One suggestion is to provide an example with something like "backport/issue_XYZ" where XYZ refers to the issue number

The release/xyz branch is short-lived, it gets auto deleted after getting merged into the base branch. Only base/version remains (and that is the actual final "release branch"), so trying to use backport/ would be yet another way, but it would still be short lived. I agree with mentioning the issue number and better commit messages, and on PR titles [release] for whatever releases and then add [backport] too if it's being backported. But, for the branch name itself I'd say let's stick with release/xyz let me know if you meant something else.

Arguably it could be clearer if our base/* branches were named release/* ones, but historically we ended up with base/*. And then our current release/* could've been like pre-release/* (short lived) but now it's probably too late to change this.

We should state on the documentation that it is allowed to have multiple PRs targetting the same backport number, although it is recommended not to do so

Yes, if they are in-flight that's OK. On base/xyz branches we never merge more than once.

--

Yes to all the other suggestions, great ones. Thanks for your inputs.

@Alopalao Alopalao linked a pull request Dec 16, 2024 that will close this issue
@viniarck
Copy link
Member Author

viniarck commented Dec 16, 2024

@italovalcy regarding e2e, we might want to start tagging a tag version for e2e tests too, just so when running with END_TO_END_BRANCH it gets tests that are supposed to still with with that version, otherwise newer tests on master might not be fully compatible although they tend to be. For unit tests, running locally with tox should suffice, and then we can also start for the tests to be backported too. Backports will require more effort, but worth it for the quality, and then we can align which issues actually deserve to be backported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation priority_low Low priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants