Description
Eric Phetteplace
2017-04-13 21:24:20 UTC
Created attachment 62155 [details] [review] patch I am not sure if we should start changing all comments for this kind of trivial edits. I would better understand changing templates (what we actually see on the Koha interface). But you seem to touch scripts and modules only. I disagree that making code base gender inclusive is trivial. It discourages contributions from female developers. It's also just flat-out inaccurate, as in places where tests assume all juveniles are male (t/db_dependent/Koha/Patrons.t). I searched over the templates as well and they tended to not have this problem, it's almost entirely in comments or strings printed during tests. QA tools complain for one file: FAIL acqui/neworderempty.pl FAIL forbidden patterns forbidden pattern: tab char (line 157) Question: Why is the male form always at the first place ('he or she' / 'him or her')? Does that not introduce a new inequality? Created attachment 62182 [details] [review] new patch Alphabetical order is one reason. As I said, I'd be happy to use singular "they/them" universally if people prefer that. I could also alternate orderings between "he or she" and "she or he". New patch with whitespace changes added. Created attachment 62184 [details] [review] Bug 18432: code comments assume male gender Comments throughout the Koha codebase assume that all librarians or borrowers are male by using the pronoun 'he' universally. This patch changes to 'he or she' / 'him or hers'. Testing plan: - ensuring modifying tests still pass: + C4/SIP/t/06patron_enable.t + t/db_dependent/Circulation.t + t/db_dependent/Koha/Patrons.t + t/db_dependent/Reserves.t Sponsored-By: California College of the Arts No code changes detected. Signed-off-by: Marc Véron <veron@veron.ch> I guess you are going to expect this to be a new coding guideline? In that case you should write a proposal and submit the topic to the next dev meeting. I personally think that "he or she" is heavy to read. The patch is already outdated t/db_dependent/Koha/Patrons.t: is( $total, $enrolmentfee_K + $enrolmentfee_J, "Kid growing and become a juvenile, he should pay " . ( $enrolmentfee_K + $enrolmentfee_J ) ); I think this is a useful contribution, however I suggest we change the pronoun to they. It is non gendered and neutral I agree, "they" is better. More concise & more inclusive. I will write a proposal to the koha-devel email list and try to make the next developer's meeting, though if I recall all the recent ones have been during my commute hour so I've never been able to attend. I assume once it's approved I am OK to add to the wiki page on coding guidelines and write a new patch that changes all instances of "he/him" and "he/him or she/her" to "they/them". (In reply to Eric Phetteplace from comment #11) > I agree, "they" is better. More concise & more inclusive. > > I will write a proposal to the koha-devel email list and try to make the > next developer's meeting, though if I recall all the recent ones have been > during my commute hour so I've never been able to attend. I assume once it's > approved I am OK to add to the wiki page on coding guidelines and write a > new patch that changes all instances of "he/him" and "he/him or she/her" to > "they/them". I'm currently just doing a follow up patch that changes the language, should be done soon, so you can have an example to show if you want. (I'll leave your original patch and do it as a follow up, so you still get credit for a commit) Created attachment 62336 [details] [review] Bug 18432 : Follow up -Updating to use they/them and skipping the ones changed to it Created attachment 62467 [details] [review] [SIGNED-OFF] Bug 18432: code comments assume male gender Comments throughout the Koha codebase assume that all librarians or borrowers are male by using the pronoun 'he' universally. This patch changes to 'he or she' / 'him or hers'. Testing plan: - ensuring modifying tests still pass: + C4/SIP/t/06patron_enable.t + t/db_dependent/Circulation.t + t/db_dependent/Koha/Patrons.t + t/db_dependent/Reserves.t Sponsored-By: California College of the Arts No code changes detected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 62468 [details] [review] [SIGNED-OFF] Bug 18432 : Follow up - Updating to use they/them Updating to use they/them and skipping the ones changed to it Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 62482 [details] [review] Bug 18432: code comments assume male gender Comments throughout the Koha codebase assume that all librarians or borrowers are male by using the pronoun 'he' universally. This patch changes to 'he or she' / 'him or hers'. Testing plan: - ensuring modifying tests still pass: + C4/SIP/t/06patron_enable.t + t/db_dependent/Circulation.t + t/db_dependent/Koha/Patrons.t + t/db_dependent/Reserves.t Sponsored-By: California College of the Arts No code changes detected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 62483 [details] [review] Bug 18432 : Follow up - Updating to use they/them Updating to use they/them and skipping the ones changed to it Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 62484 [details] [review] Bug 18432: Replace 2 'he or she' with 'they' Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Added a check to the QA tests: commit 953c8903f3d2dde89be05df6c4b0d8f671736886 Do not assume male gender, use they/them instead (bug 18432) Pushed to master for 17.05, thanks Eric, Chris, Jonathan! Doesn't apply cleanly - please provide rebased patches for stable if you think it should be in 16.11.x. |