Benni asked me to include http://tools.typo3.org/cms/merged/core.html into Forger.
I don’t know what the views are used for so I tried to understand what’s needed to write the requirements down here first.
Please come up with ideas that would make your life easier and drop old stuff you no longer use.
Requirements
Find all merged reviews aimed on branch master which has other branches in the Releases line
For those, look for equivalent reviews based on the ChangeId and the targeted branch (TYPO3_7-6 for 7.6 for example)
If a review exists and is not yet merged, link to it with the text “Review” being linked
If a review exists that has been merged, link to it with the text “Merged” being linked.
If no review exists, show a red “TODO” button
If all branches mentioned in the Releases line have merged backports, remove the patchset from the list
Supply the possibility to blacklist review IDs in case it was decided later not to backport
Great initiative! I hacked that helper back in the 4.5 time, and wondered how it was still being used. But it seems to be, as I constantly get requests to merge new “blacklist entries”. Original code currently: https://github.com/baschny/typo3-merged. There are lots of crazy heuristics in the old script which are probably not required anymore.
The requirements read good, and I think are generally enough for the Contributors / Reviewers job.
Some requirement I had back then were mostly due to my job as a release manager and active issue tracker contributor. Not sure if it still helps anyone:
I wanted to know in which patch-level release each fix was included. This was my personal interest and a helper for supporting people in forum / issue trackers. For example https://forge.typo3.org/issues/70962, I can instantly tell you this was fixed in 6.2.30 and 7.6.10. This information is very difficult to gather manually from GIT.
This also allowed me to generate this list: http://tools.typo3.org/cms/merged/core-6.2.html which is a list of issues fixed in every TYPO3 patch level release. As a release manager that was also useful (for statistics, to write some release news etc).
The “Components” part was generated from the --diff-stats and reporting which sysext was affected by the patch (and some other specific areas of the Core). This also was helpful for me to generate more detailed and categorized reports / release notes etc.
Make the currently “supported” branches configurable, to be able to skip old branches that we no longer support.
So you want to automate git describe --contains [COMMITHASH] across branches, correct?
Given the amount of releases I’d envision this not just as a plain list but as something where you select the current tag and get the result of git log 7.6.13..7.6.14 --oneline back.[quote=“baschny, post:2, topic:57”]
The “Components” part was generated from the --diff-stats and reporting which sysext was affected by the patch (and some other specific areas of the Core). This also was helpful for me to generate more detailed and categorized reports / release notes etc.
[/quote]
Frank and I built that some time back to automatically set the “topic” of gerrit patches.
It does some things like checking which extension is affected by the change, rule out language changes and test-related stuff to take a pretty solid guess… but still, it’s a guess.
I think it would make more sense to re-enable this command on a regular basis.[quote=“baschny, post:2, topic:57”]
Make the currently “supported” branches configurable, to be able to skip old branches that we no longer support.
[/quote]
Jup, that’s something I need to think about how convenient we can build this. Up until now I didn’t have a reason for user authentication within Forger, maybe I need to build it now.
For me the focus should be to replace the parts that can’t be easily replicated with git commands and are used most often, which is the list of missing backports.
The other parts of the tool are “sugar” in my eyes, which aren’t necessary for migrating the tool over to forger.
If backports are all merged the patch should not be included in this list anymore. Main use case is to provide an easy way to see which backports are not completed yet.
The “blacklist” feature is very useful indeed!
Looks good to me. The main usecase I used the tool for was finding missing backports for older branches. Removing stuff that is complete from the list makes the loading time much better than the current solution.
Can we somehow reduce the amount of merged reviews?
If we don’t do this, we know the app won’t scale anymore after a certain point.
So can we only query all closed reviews that are less than 6 months old or something?
I am uncertain if reducing the number of reviews is a good idea since it sometimes takes quite long until the backports are actually in and it would be bad if those would just not show up due to a time constraint. The amount of reviews is near the maximum right now, since when TYPO3 6.2 enters ELTS this branch would be removed from that view (at least this was the decision for 4.5 at that time - and it still makes sense IMO)
Apart from that it is a lot of data but it is the data we want to work with, so either the new tool has to cope with that or its not a good idea to switch to it.
Anyways:
What I mean is, should we be able to restrict the timeframe or anything.
Otherwise the following is going to happen:
10 years from now and 12000 reviews in, the list will simply load and load and load and load.
This does not make sense to me.
We need to be able to define when a review-set is done and should no longer be displayed by default.
Ping… Any updates on this discussion. Having a tool that shows probably forgotten back ports is essential. And having that maintained in the TYPO3 infrastructure scope would be great as well.