[I am sorry for this...] C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine!
Created attachment 31023 [details] [review] Bug 12792: C4::Reserves breaks my vim syntax color C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine! Test plan: launch git show HEAD and verify this patch won't break anything.
Diving into the wonderful world of vim configuration was no option? :)
I didn't find anything. I know this patch is quite egoist and I don't want to impose something. If someone does not agree with it, I can close this bug report. But the patch is trivial...
I don't mind this patch myself. But sorry, you modify three functions: AddReserve, GetReserveFee, and _Findgroupreserve. There are only tests for AddReserve in our current test suite. You need to improve test coverage if you want your tweaks in GetReserveFee and _Findgroupreserve to pass QA properly, in my understanding.
I am not sure to understand what I could test here...
(In reply to Jonathan Druart from comment #5) > I am not sure to understand what I could test here... Oops. Nevermind. Turns out the t/db_dependent/Reserves.t triggers them all. Sign off coming up. :)
Created attachment 33268 [details] [review] Bug 12792: C4::Reserves breaks my vim syntax color C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine! Test plan: launch git show HEAD and verify this patch won't break anything. Additionally, prove t/db_dependent/Reserve.t This will trigger the three functions that were modified. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Jonathan, the patch doesn't apply cleanly - can you take a look?
Bug 11126 is what is causing the conflict.
Created attachment 33350 [details] [review] Bug 12792: C4::Reserves breaks my vim syntax color C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine! Test plan: launch git show HEAD and verify this patch won't break anything. Additionally, prove t/db_dependent/Reserves.t This will trigger the three functions that were modified. Sorry, but t/db_dependent/Reserves.t fails currently. Checking why.
Created attachment 33351 [details] [review] [SIGNED OFF] Bug 12792: C4::Reserves breaks my vim syntax color C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine! Test plan: launch git show HEAD and verify this patch won't break anything. Additionally, prove t/db_dependent/Reserves.t This will trigger the three functions that were modified. The prove currently fails on test 8, but the other succeeding tests prove that this change is fine. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
(In reply to M. Tompsett from comment #11) > The prove currently fails on test 8, but the other succeeding > tests prove that this change is fine. Oops. Test 11. Still, the change is good and worthy of QA.
(In reply to M. Tompsett from comment #12) > (In reply to M. Tompsett from comment #11) > > The prove currently fails on test 8, but the other succeeding > > tests prove that this change is fine. > > Oops. Test 11. Still, the change is good and worthy of QA. Looks good here: prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 1/50 Subroutine C4::Context::userenv redefined at t/db_dependent/Reserves.t line 42. C4::Context->userenv not defined! at C4/Branch.pm line 143. C4::Context->userenv not defined! at C4/Items.pm line 2305. t/db_dependent/Reserves.t .. ok All tests successful. Files=1, Tests=50, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.60 cusr 0.04 csys = 1.66 CPU) Result: PASS
Created attachment 33390 [details] [review] [PASSED QA] Bug 12792: C4::Reserves breaks my vim syntax color C4/Reserves.pm is unreadable with my vim configuration. It appears I am the only one having this problem. For an incomprehensible reason, a string constructs with qq/my string/; completely breaks the syntax color for all the rest of the file (~2300l). If I replace it with qq{my string}; all is fine! Test plan: launch git show HEAD and verify this patch won't break anything. Additionally, prove t/db_dependent/Reserves.t This will trigger the three functions that were modified. The prove currently fails on test 8, but the other succeeding tests prove that this change is fine. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass on my installation. No problems found.
Patch pushed to master. Thanks Jonathan!