Bug 18936 - Circulation Rules Ratification
Hi All, I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling. It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find. Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future. Many thanks to those who have spent many hours coding, testing, refining and qaing this piece, *Martin Renvoize* <https://www.ptfs-europe.com> Development Team Manager Community Release Manager (19.11, 20.05) *Phone:* +44 (0) 1483 378728 *Mobile:* +44 (0) 7725 985 636 *Email:* martin.renvoize@ptfs-europe.com *Fax:* +44 (0) 800 756 6384 www.ptfs-europe.com Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30 The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com
Congrats to all involved! El mar., 4 feb. 2020 a las 7:02, Renvoize, Martin (< martin.renvoize@ptfs-europe.com>) escribió:
Hi All,
I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling.
It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find.
Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future.
Many thanks to those who have spent many hours coding, testing, refining and qaing this piece,
*Martin Renvoize*
Development Team Manager
Community Release Manager (19.11, 20.05)
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
Hi Martin, I just discovered this bug and, after reading quickly the bug's comments and commit messages, I didn't find the reason of this big refactoring (why was it needed ? what do we gain ? is it only a rename or is it more than that ?). Is this explained somewhere ? In my opinion changes should be summarized at least in the first commit message, so that people rebasing other patches can have an overview of what changes are needed in their patches. Le 04/02/2020 à 11:01, Renvoize, Martin a écrit :
Hi All,
I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling.
It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find.
Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future.
Many thanks to those who have spent many hours coding, testing, refining and qaing this piece,
*Martin Renvoize*
Development Team Manager
Community Release Manager (19.11, 20.05)
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com <mailto:martin.renvoize@ptfs-europe.com>
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com <https://www.ptfs-europe.com>
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com <mailto:info@ptfs-europe.com>
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre
Bug 18936 is the groundwork for bug 15522 (revamp issuing rules interface). Everybody should have heard about it as the mailing list contains several occurrences of this work: ask for help, signoff/QA, as well as the different call for the proposal, starting 4 years ago. Bug 18936 does not bring anything to the end-user, but basically you just need to keep in mind that: - the issuingrules table does not longer exist (!) - rules are now in the circulation_rules, which has the following structure: branchcode, categorycode, itemtype, rule_name, rule_value (so 1 row per rule) - you should call Koha::CirculationRules->set_rule (single) or set_rules (multiple) to define/set circulation rules - the rebase was painful and I apologized in advance for the issues I left behind :) - bug 15522 is on its way, and it would be really useful to get more testing on it! Cheers Le mar. 4 févr. 2020 à 14:25, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi Martin,
I just discovered this bug and, after reading quickly the bug's comments and commit messages, I didn't find the reason of this big refactoring (why was it needed ? what do we gain ? is it only a rename or is it more than that ?). Is this explained somewhere ?
In my opinion changes should be summarized at least in the first commit message, so that people rebasing other patches can have an overview of what changes are needed in their patches.
Le 04/02/2020 à 11:01, Renvoize, Martin a écrit :
Hi All,
I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling.
It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find.
Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future.
Many thanks to those who have spent many hours coding, testing, refining and qaing this piece,
*Martin Renvoize*
Development Team Manager
Community Release Manager (19.11, 20.05)
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com <mailto:martin.renvoize@ptfs-europe.com>
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com <https://www.ptfs-europe.com>
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com <mailto:info@ptfs-europe.com>
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Thanks for the clarification, but my comment was more about "how people could find this information" rather than "give me this information". Currently it's not in the bug, not in commit messages, not in any kind of documentation. During the last developer irc meeting it was decided to reinforce good commit message guideline, and I think that could have been useful here. Specifically the "why" part is missing and the "what" part could have been more verbose. Apart from this particular bug, I think everyone would benefit from a "developer changelog" where all this kind of changes (database renames, subroutines removals, ...) would be written. Just my 2 cents... PS: Not everyone has time to look at all the bugs discussed on the mailing list. Please don't make me feel guilty about it :) Le 04/02/2020 à 16:40, Jonathan Druart a écrit :
Bug 18936 is the groundwork for bug 15522 (revamp issuing rules interface).
Everybody should have heard about it as the mailing list contains several occurrences of this work: ask for help, signoff/QA, as well as the different call for the proposal, starting 4 years ago.
Bug 18936 does not bring anything to the end-user, but basically you just need to keep in mind that: - the issuingrules table does not longer exist (!) - rules are now in the circulation_rules, which has the following structure: branchcode, categorycode, itemtype, rule_name, rule_value (so 1 row per rule) - you should call Koha::CirculationRules->set_rule (single) or set_rules (multiple) to define/set circulation rules - the rebase was painful and I apologized in advance for the issues I left behind :) - bug 15522 is on its way, and it would be really useful to get more testing on it!
Cheers
Le mar. 4 févr. 2020 à 14:25, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi Martin,
I just discovered this bug and, after reading quickly the bug's comments and commit messages, I didn't find the reason of this big refactoring (why was it needed ? what do we gain ? is it only a rename or is it more than that ?). Is this explained somewhere ?
In my opinion changes should be summarized at least in the first commit message, so that people rebasing other patches can have an overview of what changes are needed in their patches.
Le 04/02/2020 à 11:01, Renvoize, Martin a écrit :
Hi All,
I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling.
It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find.
Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future.
Many thanks to those who have spent many hours coding, testing, refining and qaing this piece,
*Martin Renvoize*
Development Team Manager
Community Release Manager (19.11, 20.05)
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com <mailto:martin.renvoize@ptfs-europe.com>
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com <https://www.ptfs-europe.com>
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com <mailto:info@ptfs-europe.com>
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre
Hi, if you are still interested to know the why part, it is explained in the wiki: <https://wiki.koha-community.org/wiki/Circulation_Rules_Interface_and_Backend_Revamp_RFC>. I definitely agree it should have been in the commits too but I got maybe too lax as a QA person since we had so many regressions fixed already (took around 2 weeks to do the QA) the earliest patches were send and revised already 2.5 years ago so this has been a long process but I guess not a real excuse either. As for the developer changelog, I'm fine with reading just the git history but for this to work ideally I still think the follow-ups from the patch author itself are bad and we need to be strict about enforcing good commit messages! On 04/02/2020 18:58, Julian Maurice wrote:
Thanks for the clarification, but my comment was more about "how people could find this information" rather than "give me this information". Currently it's not in the bug, not in commit messages, not in any kind of documentation.
During the last developer irc meeting it was decided to reinforce good commit message guideline, and I think that could have been useful here. Specifically the "why" part is missing and the "what" part could have been more verbose.
Apart from this particular bug, I think everyone would benefit from a "developer changelog" where all this kind of changes (database renames, subroutines removals, ...) would be written.
Just my 2 cents...
PS: Not everyone has time to look at all the bugs discussed on the mailing list. Please don't make me feel guilty about it :)
I can easily understand why the commit messages were not revised, and I certainly wouldn't want to be the person that delay such an important patchset because of the commit messages. The problem with commit messages is that they cannot be changed/improved after the commits have been pushed. And as you said, follow-ups often pollute the history. Also, it may be easier to enforce a rule like "you have to write one line in ChangeLog" than a rule like "you have to write a good commit message" because good and bad are really subjective. And if the line in ChangeLog is not good enough, it can still be improved later, after the commits are pushed. Maybe we can continue this discussion on bug 22890, as Jonathan suggested Le 05/02/2020 à 09:47, Joonas Kylmälä a écrit :
Hi,
if you are still interested to know the why part, it is explained in the wiki: <https://wiki.koha-community.org/wiki/Circulation_Rules_Interface_and_Backend_Revamp_RFC>. I definitely agree it should have been in the commits too but I got maybe too lax as a QA person since we had so many regressions fixed already (took around 2 weeks to do the QA) the earliest patches were send and revised already 2.5 years ago so this has been a long process but I guess not a real excuse either.
As for the developer changelog, I'm fine with reading just the git history but for this to work ideally I still think the follow-ups from the patch author itself are bad and we need to be strict about enforcing good commit messages!
On 04/02/2020 18:58, Julian Maurice wrote:
Thanks for the clarification, but my comment was more about "how people could find this information" rather than "give me this information". Currently it's not in the bug, not in commit messages, not in any kind of documentation.
During the last developer irc meeting it was decided to reinforce good commit message guideline, and I think that could have been useful here. Specifically the "why" part is missing and the "what" part could have been more verbose.
Apart from this particular bug, I think everyone would benefit from a "developer changelog" where all this kind of changes (database renames, subroutines removals, ...) would be written.
Just my 2 cents...
PS: Not everyone has time to look at all the bugs discussed on the mailing list. Please don't make me feel guilty about it :)
-- Julian Maurice BibLibre
I am going to support any good ideas from bug 22890 (Add a chronological changelog). Still waiting for continuing the discussion there however. What can we do to move it forward? Le mar. 4 févr. 2020 à 17:58, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Thanks for the clarification, but my comment was more about "how people could find this information" rather than "give me this information". Currently it's not in the bug, not in commit messages, not in any kind of documentation.
During the last developer irc meeting it was decided to reinforce good commit message guideline, and I think that could have been useful here. Specifically the "why" part is missing and the "what" part could have been more verbose.
Apart from this particular bug, I think everyone would benefit from a "developer changelog" where all this kind of changes (database renames, subroutines removals, ...) would be written.
Just my 2 cents...
PS: Not everyone has time to look at all the bugs discussed on the mailing list. Please don't make me feel guilty about it :)
Le 04/02/2020 à 16:40, Jonathan Druart a écrit :
Bug 18936 is the groundwork for bug 15522 (revamp issuing rules interface).
Everybody should have heard about it as the mailing list contains several occurrences of this work: ask for help, signoff/QA, as well as the different call for the proposal, starting 4 years ago.
Bug 18936 does not bring anything to the end-user, but basically you just need to keep in mind that: - the issuingrules table does not longer exist (!) - rules are now in the circulation_rules, which has the following structure: branchcode, categorycode, itemtype, rule_name, rule_value (so 1 row per rule) - you should call Koha::CirculationRules->set_rule (single) or set_rules (multiple) to define/set circulation rules - the rebase was painful and I apologized in advance for the issues I left behind :) - bug 15522 is on its way, and it would be really useful to get more testing on it!
Cheers
Le mar. 4 févr. 2020 à 14:25, Julian Maurice <julian.maurice@biblibre.com> a écrit :
Hi Martin,
I just discovered this bug and, after reading quickly the bug's comments and commit messages, I didn't find the reason of this big refactoring (why was it needed ? what do we gain ? is it only a rename or is it more than that ?). Is this explained somewhere ?
In my opinion changes should be summarized at least in the first commit message, so that people rebasing other patches can have an overview of what changes are needed in their patches.
Le 04/02/2020 à 11:01, Renvoize, Martin a écrit :
Hi All,
I just wanted to let you all know that I'll be pushing bug 18936 to the master branch this morning. It's the largest to date of the refactoring bugs and will affect any/all bugs related to the circulation rules handling.
It's been thoroughly tested and QA'd, but as a large piece I'm sure there will be a few pain points still buried, so keep an eye out and report any bugs you find.
Good luck with any rebases that may be required after this and my apologies, but the end result should give us a much cleaner foundation for circ related enhancements and cleanup in the future.
Many thanks to those who have spent many hours coding, testing, refining and qaing this piece,
*Martin Renvoize*
Development Team Manager
Community Release Manager (19.11, 20.05)
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com <mailto:martin.renvoize@ptfs-europe.com>
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com <https://www.ptfs-europe.com>
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com <mailto:info@ptfs-europe.com>
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Julian Maurice BibLibre
participants (5)
-
Jonathan Druart -
Joonas Kylmälä -
Julian Maurice -
Renvoize, Martin -
Tomas Cohen Arazi