https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38664
Victor Grousset/tuxayo <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #17 from Victor Grousset/tuxayo <[email protected]> --- IIUC we are loosing more time constantly due to: - making tidying commits themselves - QA having to find which code section is the QA script complaining about if the QAer is fixing it themselves - same for the submitter for fixing it - another round trip of fail QA and back to signed off - the conflicts these commits cause with other submitted patches on close parts of the code when those patches are not tidied yet - and the backporting issues when there is a follow-up ticket built on tidied code but whose tidy commit had to be skipped. (so 2 bugs touching the same code and the tidy of 1st one wasn't kept to avoid conflict but now that causes conflicts when backporting the 2nd bug) All this, which is a constant burden vs the time lost to - take one massive round of conflicts with the submitted patches pool - and the backporting hassle significant for like 2 cycles. (stable and oldstable RMaints will need to ask a lot of rebasing help otherwise it will be impossible to keep up) So unless something makes the scale tilt the other way, let's grit our teeth and go for it! :) ----- To facilitate fixing conflicts on submitted patches, there might be this technique: - list the files giving conflicts - abort the failed apply/rebase operation - rebase the patches on the latest commit before the tidy patch (a big reason to tidy pl, js, tt, etc at the same time) - tidy the files that conflicted (the exact tidying commands should be somewhere easy to be found when needed) - commit - squash with the commit containing the actual changes - when there are multiple patches, they would all need to be squashed together :/ (any way around this?) - rebase on main to apply the great tidying patches and the rest - hopefully there shouldn't be conflicts (to be confirmed ^^") except regular legit ones And maybe that could be a base for the script to ease rebases. Hopefully there is a better strategy? (either for a script of manual step) So people with pending patches have the easiest time to get them though the great tidying. -------- @all Is there anything important that should be changed in the .perltidyrc and .prettierrc.js ? It's now or never. (we don't have a .tt tidying tool already, do we?) ------- (In reply to Jonathan Druart from comment #0) > I am suggesting to have patches for 25.05 that will tidy our perl (pl, pm, > t), javascript (js, ts, vue) and template (tt, inc) files. When testing Bug 38524 and it's dependencies, I don't recall prettier particularly complaining about the various ts and vue files I tested. So our config looks good. Because as ts and vue files are recent, prettier shouldn't have much to say if it's config is in line with out recent practices. So no issue foreseen there 👍 This is opposed to ESLint. Which raises a lot of issues in our ts and vue files. So the config doesn't match out recent practices and both need to be reconciled because it's not clear which of the two is right for us. So good thing it's only about Prettier. @all That's still an occasion to make important changes to the prettier config if needed. Note that Prettier should be on a recent version. Because on main, without Bug 38149, prettier is on whatever arbitrary version was latest when it was installed on one's dev env. Since it was not in package.json. --------- (In reply to Katrin Fischer from comment #13) > For info: this RM runs xt tests before pushing :) Should it be explicitly in the RM process? That's the closest thing I've found: https://wiki.koha-community.org/wiki/Release_management#Checking_for_regressions But it's not about running the whole xt/ tests. But maybe a pre-push hook might be enough for the RM. Or it's better to run all xt/ ? ------------------ (In reply to Jonathan Druart from comment #14) > The idea (not tested yet and needs to be confirmed/improved/discussed) is to: > 0. have a patch set for bug 12345 that has been written before the tidy > commit(s) > 1. run "the rebase script" > a. checkout commit before the tidy bug > b. apply patches from 12345 one-by-one > c. run the tidy script on each of the commit > d. rebase and keep changes from 12345: git checkout --theirs (or git > rebase -Xtheirs) > 2. Attach the rebased version of the patches c. is not possible. If it tidies the 1st commit, then it wouldn't be able to apply the 2nd anymore. The start point of the diff would have changed. That's why in my attempt earlier in this message, I had to resolve to tidy only after applying all commits and squash all :( Then apply the tidy bug. Wait, thinking about it again, there might be a way: a. checkout commit before the tidy bug b. apply the 1st patch c. tidy, commit and squash d. in a temporary branch (starting like step a.): apply the first two commits e. tidy, commit and squash f. now it should work to do `git merge --squash temp-branch;git commit` (and find how to reuse the commit message of commit 2) --squash takes the diff of two branches and prepares it to be committed. All this to have only the changes from commit 1 to commit 2 as if they where both tidied when authored. Otherwise commit 2 directly would conflict or rollback part of tidying of commit 1. And likely create invalid code. Like when tidying spreads stuff on multiple lines. 💥 Then repeat with a temporary branch having commit 1 + 2 + 3 and then tidy and squash. Any less contrived way to get the same result? 😵💫😵💫😵💫 ------------------------ (In reply to Mark Hofstetter from comment #16) > one question for me is what happens to all bugfixes waiting for testing & > QA, theoretically the should all be tidy or not? > > if not they maybe should also be titied before automatically? They are targeting the current code. So their diffs are from a non tidy codebase. Even if they would contain tidying changes (often it's in the last commit so the first commits don't have tidying) the starting point of the diff would have changed. So when trying to apply to the future tidied codebase git won't be able to match where to apply. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
