https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39756 --- Comment #18 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- (In reply to Philip Orr from comment #17)
Hi Chloé, I'd like to test this one but I'm not sure about the best procedure to test it. It depends on 43500, 43504 and 43508 - would it be better to test those first and then come back to this one?
I tried applying this bug (that is, 39756) and failed at 43508:
``` 📋 Bug 43508 - Extract Koha::Account->add_lost_replacement_fee from chargelostitem
• 207940 - Bug 43508: refactor(Account): extract add_lost_replacement_fee • 207941 - Bug 43508: test(Account): add_lost_replacement_fee • 207942 - Bug 43508: (follow-up) Remove non-ASCII character from add_lost_replacement_fee POD
Apply? [(y)es, (n)o, (i)nteractive] y
Preparing 3 patch(es): ✓ Prepared 3 patch(es)
Applying 3 patch(es):
Patch application failed for attachment 207940 [details] [review] - Bug 43508: refactor(Account): extract add_lost_replacement_fee
Patches left in /tmp/XCidSfC3J1 for manual application if needed
To resolve: 1. Fix conflicts (use 'git mergetool' or edit files manually) 2. Stage resolved files with 'git add' 3. Continue with 'git bz apply --continue' or 'git am --continue' 4. Or skip this patch with 'git bz apply --skip' 5. Or abort with 'git bz apply --abort'
Error: Apply failed: Cannot apply cleanly patches from bug 43508. Everything will be left dirty. Git command (git am -3 /tmp/XCidSfC3J1/0001-207940.patch) failed: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch hint: When you have resolved this problem, run "git am --continue". hint: If you prefer to skip this patch, run "git am --skip" instead. hint: To restore the original branch and stop patching, run "git am --abort". hint: Disable this message with "git config advice.mergeConflict false" ´´´
Hi Philip, Thanks for this, much appreciated :) Thinking about it, the issue here is probably that 43508 conflicts with 43504. Both introduce a new method in a similar location in Account.pm. And both add a test to Account.t, so that's likely complaining too. One solution could be that I make 43504 depend on 43508, so that it includes that merge conflict resolution. (Rather than keep 43504 compatible with current main). The reason I would do it that way is 43508 already has one sign off, so I reckon it might be slightly better if 43504 is the one I rebase. I wasn't sure this was a great idea seeing as there's no functional dependency between the two. But it would make testing 39756 itself much easier. If you're happy with it, I can go ahead and sort 43504 now. So as for how to proceed in general, I think the sequential, bug by bug approach is our best bet here. Thanks again! -- You are receiving this mail because: You are watching all bug changes.