devguide: discuss backports and other updates - v1 #9884
Conversation
This could be justified from a semantic point of view, and also can help in bringing more attention to where this information is, as it is less hidden, now.
This section seemed to aim both at PR reviewers and PR authors at the same time, even though some info is probably of low value for contributors. Created new section for PR reviewers and maintainers, and kept the info for PR authors separated. Also highlighted information on requested changes and stale PRs.
If a commit introduces code that changes Suricata behavior, we understand that the documentation changes should go in the same commit. Among other things, this reduces the chances of said changes being lost if there are backports or many work versions before approval.
Update the list of active branches to include 7 renaming and new master, link to backports document.
Sphinx and RtD sometimes render lists in weird ways. The communication channels list barely looked like one, at all...
| cherry-picked commit. | ||
| * *Add additional commits*, if any are needed (e.g., to adjust cherry-picked code | ||
| to old behavior). | ||
|
|
There was a problem hiding this comment.
Its not often, but sometimes something was rewritten in the latest version but the old version still needs to be fixed. In this case there are no commits to backport.
There was a problem hiding this comment.
Wondering if this belongs to this section, or to the part about creating backport tickets. Do I understand this correctly that this is about, for instance, something that has changed from 6 to 7, and was now fixed in 8, so there could be a backport for 7, but not for 6?
There was a problem hiding this comment.
Wondering if this belongs to this section, or to the part about creating backport tickets. Do I understand this correctly that this is about, for instance, something that has changed from 6 to 7, and was now fixed in 8, so there could be a backport for 7, but not for 6?
No. I'm thinking more about a bug that is found to affect 7 and git master, but where cherry-pick ID's don't really make sense. A good example would be the case where something was rewritten in Rust. In master you'd be doing a patch in Rust, and in 7 you'd be doing a patch in C. Refactors, etc. can also result in this.
There was a problem hiding this comment.
Ah, thanks, got it, I'll see how to incorporate that.
| *(7.0.x-backport)*. | ||
|
|
||
| In the PR description, indicate the backport ticket. | ||
|
|
There was a problem hiding this comment.
Do we, should we use the milestone feature in GitHub?
| @@ -13,14 +13,15 @@ developed by the [OISF](https://oisf.net) and the Suricata community. | |||
| - [Home Page](https://suricata.io) | |||
| - [Bug Tracker](https://redmine.openinfosecfoundation.org/projects/suricata) | |||
| - [User Guide](https://docs.suricata.io) | |||
| - [Dev Guide](https://docs.suricata.io/en/latest/devguide/index.html#suricata-developer-guide) | |||
There was a problem hiding this comment.
I think we can remove the hash part of this URL.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9884 +/- ##
==========================================
- Coverage 82.45% 82.45% -0.01%
==========================================
Files 973 973
Lines 273063 273063
==========================================
- Hits 225155 225143 -12
- Misses 47908 47920 +12
Flags with carried forward coverage won't be shown. Click here to find out more. |
| Suricata Backports Guide | ||
| ======================== | ||
|
|
||
| This document describes the processes used to backport content to previous |
| * Current release | ||
| * Release prior to current release |
There was a problem hiding this comment.
Not sure about the words here. Perhaps something like
- master
- major stable 1 ...
| For example, at the moment, there are 3 releases based on these Suricata branches: | ||
| * master: 8.0.0-dev, current development branch | ||
| * main-7.0.x: current stable release (note we're changing our naming conventions) | ||
| * master-6.0.x: previous stable release |
There was a problem hiding this comment.
Do we plan on upgrading this doc everytime we EOL a branch or create a new one?
There was a problem hiding this comment.
Good question. Considering it's something to be done every... two or three years, I don't think it's a bad idea to review/ check for needed updates at least then. imho.
| Usually, when the team creates a ticket, we'll add the *Needs backport* related | ||
| labels, so necessary backporting tickets will be automatically created. If you | ||
| are working on a ticket that doesn't have such labels, nor backporting tasks | ||
| associated, it probably doesn't need backporting. But sometimes we'll miss those. |
There was a problem hiding this comment.
or if they think it's something that should be backported, they should comment on the ticket.
| There can be cases where backports may be "missed" -- some issues may not be | ||
| labeled as needing backports and some PRs may be merged without an issue. |
There was a problem hiding this comment.
Perhaps, these should go in an Exceptions or something section at the end? Thoughts?
There was a problem hiding this comment.
I'll add a note in this section, I think it's easier to keep context, like that. 🤔
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
| Redmine: when creating a new redmine issue, label the Redmine issue with "needs | ||
| backport to x.y.z”, where x.y.x is a supported Suricata release, e.g, 7.0.x, |
| the commits into master and select only the commits from the issue being | ||
| backported. | ||
| * *Bring each commit into the new branch,* one at a time -- starting with the oldest | ||
| commit. Use ``git cherry-pick -x commit-hash`` as it maintains the |
There was a problem hiding this comment.
Maybe add something like "where commit-hash is the commit from master that is being backported"
There was a problem hiding this comment.
good catch, thanks!!
|
Feedback incorporated on: #9914 |
Link to redmine ticket:
Describe changes:
working with the codebasechapter.-or*seems to impact how they are rendered?Result can be seen at: https://suri-rtd-test.readthedocs.io/en/contribution-docs-updates-v1/devguide/contributing/backports-guide.html