15:01:10 <cait> #startmeeting Developer IRC Meeting, 2 February 2016
15:01:10 <huginn`> Meeting started Tue Feb  2 15:01:10 2016 UTC.  The chair is cait. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:01:10 <huginn`> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
15:01:10 <huginn`> The meeting name has been set to 'developer_irc_meeting__2_february_2016'
15:01:18 <cait> #topic Introductions
15:01:18 <wahanui> #info wahanui, a bot that has become sentient
15:01:30 <cait> Please introduce yourself using #info like wahanui just did
15:01:39 <cait> #info Katrin Fischer, BSZ, Germany
15:01:46 <oleonard> #info Owen Leonard, Athens County Public Libraries
15:01:52 <pianohacker> #info Jesse Weaver, ByWater Solutions, USA
15:02:02 <cait> #link https://wiki.koha-community.org/wiki/Development_IRC_meeting_2_February_2016
15:02:05 <talljoy> #info Joy Nelson, ByWater Solutions, USA
15:02:16 <jajm> #info Julian Maurice, BibLibre, France
15:02:18 <Joubu> #info Jonathan Druart
15:02:25 <matts> #info Matthias Meusburger, Biblibre, France
15:02:32 <marcelr> #info Marcel de Rooy, Rijksmuseum, Netherlands
15:02:35 <andreashm> #info Andreas Hedström Mace, Stockholm University Library
15:02:50 <bag> #info Brendan Gallagher ByWater
15:03:03 <cc> #info Colin Campbell, PTFS-Europe
15:03:30 <kidclamp> #info Nick Clemens, ByWater Solutions, USA
15:03:37 <drojf2> #info mirko tietgen, not really here
15:03:55 <tcohen> #info Tomas Cohen Arazi, Theke
15:04:28 <khall> #info Kyle M Hall, Bywater Solutions
15:04:29 <NateC> #info Nate Curulla, BWS
15:04:30 <bag> hi mirko not really here :D
15:04:51 <cait> before we start with the topics on the wiki - any announcements?
15:05:02 <cait> RM notes? :)
15:05:23 <bag> Currently few notes
15:05:42 <bag> pushed the XSS so please test test that and let’s see if we can find any missing spots
15:06:00 <bag> the web-installer needs some touch up - but I think mtompset caught some of that
15:06:15 <Joubu> (there is one already, patched, passed qa)
15:06:23 <cait> #info XSS patches were pushed, please test and help to find any remaining problems
15:06:24 <bag> yes :)  awesomeness
15:06:39 <bag> Still looking for more testers of Elastic Search
15:07:02 <cait> bag: which branch is recommended for testing?
15:07:07 <cait> catalyst or kc?
15:07:11 <Joubu> kc
15:07:12 <bag> there is a branch in kc
15:07:15 <bag> kc :)
15:07:20 <indradg> #info Indranil Das Gupta, L2C2 Technologies
15:07:31 <cait> #info Elastic search needs testing as well - please use the branch on the main repository
15:07:40 <tcohen> instructions?
15:07:40 <wahanui> instructions are coming right now to the wiki near you or at http://i.imgur.com/oyZhY.jpg
15:07:49 * bag will catch up with tcohen sometime this week to get some more lessons - especially with jenkins and packaging
15:07:56 <cait> I think there is a page on the wiki with some notes at least
15:08:04 <tcohen> thanks cait
15:08:11 <bag> so that I start to hopefully clean up the tests and get master to be passing stable on jenkins
15:08:31 <cait> #link https://wiki.koha-community.org/wiki/Elasticsearch
15:08:34 <Joubu> the branch is named remotes/origin/new_12478_elasticsearch
15:08:40 <bag> yes ^^
15:08:54 <cait> #info Elastic search branch: remotes/origin/new_12478_elasticsearch
15:09:01 <bag> that is it for me - I will keep pushing what I see from PQA
15:09:14 <cait> ok, let's move to our first big topic
15:09:23 <bag> if I miss something - just shoot me an @later and I’ll get on it
15:09:30 <cait> #topic Review of Coding guidelines
15:09:46 <cait> I have tried to group the various topics a bit - I hope people are ok with it :)
15:10:11 <cait> we have a lot to get through in this meeting, for some topics i think we can only start discussion and will need a draft to vote on next
15:10:46 <cait> I will go through them by sequence, but we have to watch the time a bit today with our long agenda
15:10:51 <cait> let's start with the first item
15:11:04 <cait> #info [marcelr] In relation to Plack: Should we add a PERL rule that  prohibits defining lexical variables (my $var) in MODULES at the  outermost block (file level), and also prohibits directly accessing file  level lexicals in subroutines of SCRIPTS.
15:11:31 <marcelr> first part is less mandatory than the second part, i guess
15:11:34 <cait> I'd suggest that ideally all additions to the coding guidelines should include a good/bad example if possible
15:11:39 <ashimema> ooh.. meeting
15:11:50 <ashimema> #info Martin Renvoize, PTFS Europe
15:12:11 <cait> i am a bit out of my depth here - any comments, questions?
15:12:49 <marcelr> the first part needs some common sense
15:12:52 <cc> It would be more useful yo explain the rational behind this than to make it a rule it dosent apply to scripts which will never be used by plack
15:12:57 <bag> examples are good :)
15:12:58 <tcohen> I agree with the proposal, as it is also a clearer way to program
15:13:08 <tcohen> :-P
15:13:26 <khall> I totally agree
15:13:28 <Joubu> I almost agree with everything too
15:13:29 <cait> ok, so shoudl we note exceptions?
15:13:30 <tcohen> who likes global variables in modules?
15:13:39 <cait> or a scope where this applies?
15:13:43 <Joubu> There is no new things, we just need to update the wiki page
15:13:44 <cc> A good guideline would be to discourage globals where possible
15:14:13 <pianohacker> cc: to clarify, by "scripts which will never be used by plack", do you mean things like cronjobs?
15:14:23 <cc> yes
15:14:59 <cait> would someone be willing to work out a draft?
15:15:06 <jajm> i believe global variables can't be completely avoided in modules (for instance $VERSION, @EXPORT, ...), so why forbid it ?
15:15:23 <marcelr> obviously
15:15:36 <pianohacker> jajm: those are read-only, though, correct?
15:15:42 <tcohen> jajm: but they are not storing state
15:15:55 * ashimema has gotta go collect kids.. sorry for me absence.. again!
15:15:56 <barton> #info Barton Chittenden, bws, Louisville, KY, USA
15:16:02 <cc> no they can be manipulated at runtime
15:16:05 * ashimema will read the minutes
15:16:15 <pianohacker> cc: huh?
15:16:36 <jajm> pianohacker, tcohen, ok, so forbid global variables that are storing state ?
15:16:43 <cc> VERSION and EXPORT are not readonly
15:17:35 <tcohen> cc: forbid touching VERSION and EXPORT could be another rule :-P
15:17:42 <cait> again, someone willing to make a draft? I am eager on an #action :)
15:17:49 <pianohacker> jajm: I'd say so. Forbid state globals in modules/CGI scripts as it causes issues with plack, and discourage them in other places for style reasons?
15:18:13 <cc> lots of modules thouch EXPORT for good reasons
15:18:38 <jajm> that sounds right pianohacker
15:18:43 <marcelr> i think that we all understand exceptions for EXPORT etc.
15:18:46 * cait volunteers pianohacker
15:19:01 <cc> Document what the issue is with plack lets try and minimise "magick"
15:19:11 <cait> pianohacker: can you wite a draft that we can review later?
15:19:25 <pianohacker> cait: does the above work, or are you talking in the form of a coding guideline
15:19:26 <tcohen> cc: +1
15:19:31 <pianohacker> cait: either way, sure
15:19:32 <marcelr> cait: i think we already had a draft
15:19:34 <cait> a coding guideline: :)
15:20:16 <cait> marcelr: hm?
15:20:17 <pianohacker> I volunteer
15:20:30 <cait> just to summarize the discussion here in a short form
15:20:35 <marcelr> from the agenda
15:21:05 <pianohacker> cc: https://perl.apache.org/docs/general/perl_reference/perl_reference.html#my____Scoped_Variable_in_Nested_Subroutines <- for context
15:21:28 <cait> marcelr: I thought adding the addtional remarks to it as discussed
15:21:41 <marcelr> ok
15:22:12 <pianohacker> #action pianohacker will draw up a draft of a coding guideline regarding global variables in modules/CGI scripts, see https://perl.apache.org/docs/general/perl_reference/perl_reference.html#my____Scoped_Variable_in_Nested_Subroutines for context
15:22:31 <cait> thx
15:22:36 <cait> moving on to the next item for now
15:23:05 <cait> [marcelr] What about DBIx, Koha::Object versus old school MySQL statements in code ?  [khall] I think the use of direct DBIx should be deprecated in  favor of Koha::Object, and direct sql statements should be limited to  queries that don't work well with the object model
15:23:34 <cait> there are some rules about use of SQL in the current coding guidelines
15:23:42 <pianohacker> I think the above has been so strongly enforced on new patches that it's a de facto coding guideline
15:24:05 <cait> hm in part
15:24:16 <cait> the problem is that we have patches of different 'age' sitting in the queue
15:24:56 <cait> i think we can say DBIx before SQLfor sur, but not sure about enforcing Koha::Object for everything
15:25:05 <tajoli> #info Zeno Tajoli, CINECA, Italy
15:25:07 <khall> cait: I think we should add a grandfather clause for all new rules
15:25:13 <cait> khall: ?
15:25:16 <khall> to give them some leeway
15:25:23 <cc> I'm very unpersuaded by Koha::Objects in their current form
15:25:27 <cait> can you quickly explain a grandfather clause? :)
15:25:36 <pianohacker> agreed, there's a lot of precedent for that
15:25:42 <khall> cait: meaning these new rules affect only patches submitted after the rules were voted in
15:25:57 <khall> for older patches, we can give them more latitude
15:26:02 <barton> grandfather clause: a case that violates the proposed rules, but will be allowed because it's already there.
15:26:03 <Joubu> it's not possible to force people using dbix::class, but we get more and more module using Koha::Objects in the Koha namespace
15:26:12 <Joubu> it will be more and more easier for devs to use it
15:26:19 <marcelr> we had this discussion before about allowing DBIx in all modules or not
15:26:22 <Joubu> especially because we will get ot of example
15:26:26 <Joubu> lot*
15:26:45 <khall> marcelr: Koha::Object(s) is what came out of that discussion
15:26:57 <cait> hm we only have this in the coding guidelines: PERL19: The use of C4::SQLHelper module is deprecated   C4:SQLHelper has been superseded by DBIC (examples), please use this instead.
15:27:05 <cait> the lin goes here: https://wiki.koha-community.org/wiki/Examples_of_DBIC_in_Koha
15:27:06 <marcelr> i remember that this was not a decision
15:27:10 <tcohen> i'd leave it as a de-facto coding guideline, and let the RM have the last call on each situation
15:27:17 <pianohacker> yup, we need something stronger
15:27:31 <Joubu> cait: this page has not been updated
15:27:50 <cait> Joubu: correct - so that's not ideal... but i suggest removing the PERL19 entirely
15:27:52 <khall> tcohen: that's the situation we should avoid. It's better to give devs the info they need to write it correctly the first time
15:28:01 <pianohacker> what khall said
15:28:03 <cait> SQLHelper is no longer part of the codebase
15:28:21 <cait> cc: can you detail the problems you encountered ?
15:28:26 <tcohen> pianohacker: khall: I agree
15:28:40 <Joubu> PERL18 a PERL19 should be removed, yes
15:28:45 <Joubu> and*
15:28:49 <khall> agreed
15:29:04 <tcohen> +1
15:29:06 <cait> can we agree on removing those right away?
15:29:17 <marcelr> +1
15:29:20 <cait> one is C4::Dates deprecated, the other SQL::Helper
15:29:21 <cait> +1
15:29:26 <pianohacker> +1
15:29:29 <khall> +1
15:29:30 <Joubu> +1
15:29:34 <tajoli> +1
15:29:35 <jajm> +1
15:29:40 <barton> +1
15:29:47 <kidclamp> +1
15:29:54 <talljoy> +1
15:29:59 <indradg> +1
15:30:04 <cc> Performance, aldo negates some of the benefits of DBIIx::Class by obscuring the interface to the returned objects with an extra layer - it seems to add code rather than functionality
15:30:05 <Joubu> (add PERL11 to the list)
15:30:08 <cait> #agreed PERL18 and PERL19 to be removed, as the deprecated modules are no longer part of the codebase (C4::Dates and SQLHelper)
15:30:33 <cait> actually removing ws a later topic, i just jumped into as we were already discussing it
15:30:37 <bag> +1
15:30:44 <cait> let me read back a second for the dbix discussion
15:31:31 <khall> Joubu is right about PERL11
15:31:36 <cait> I am not sure if I remember correctly, but I think we didn't vote to enforce Koha::Object and agreed not to use DBIx in .pl files directly
15:31:45 <pianohacker> And PERL12, by its own admission?
15:32:00 <khall> pianohacker: seems that way
15:32:03 <marcelr> cait: maybe add the DBIx rule with some clause that we need good arguments to bypass it or so
15:32:26 <cait> marcelr: sorry, not sure i understand - the rule about not using in .pl files?
15:32:41 <marcelr> the rule under discussion
15:32:53 <pianohacker> cait: Perhaps to enforce usage where a Koha::Object already exists, to match what is current QA practice?
15:33:06 <cait> I am sorry, I got a bit lost
15:34:00 <pianohacker> cait: for context: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14659#c4
15:34:01 <huginn`> 04Bug 14659: enhancement, P5 - low, ---, jweaver, Needs Signoff , Allow patrons to enter card number and patron category on OPAC registration page
15:34:06 <cait> I think there are still some doubts about Koha::Object - see cc's comment, so giving some room to move would be good
15:34:53 <cait> so where a Koha::Object module exists - use that?
15:34:55 <khall> I'd like to understand cc's doubts. I don't think we've heard anything against from anyone else
15:34:58 <pianohacker> not to pick on Joubu, just to point out what I mean
15:35:05 <pianohacker> cait: yes
15:35:10 <Joubu> cait: yes, everybody has doubts about Koha::Object, but nobody never provided either a an alternative or valid arguments not to use it
15:35:19 <khall> The defacto recently has been to use Koha::Object(s)
15:35:44 <bag> I was under that thought too - the defacto
15:35:59 <marcelr> it was not decided yet
15:36:14 <cc> I've never seen a convincing rationale for using them
15:36:18 <Joubu> you can refer to "Koha and DBIC" from Septembre 2014 on Koha-devel
15:36:40 <marcelr> that discussion never reached a good conclusion iirc
15:36:41 <Joubu> then "Koha::Object" on July 2015
15:37:16 <marcelr> i would prefer a rule now with an escape for convincing arguments to qa/rm
15:37:31 <khall> marcelr++
15:37:41 <pianohacker> marcelr: basically every rule has that escape, out of necessity, but I agree :)
15:37:54 <cait> maybe we coudl summarize
15:37:57 <marcelr> pianohacker: nobody tends to provide the args
15:38:01 <cait> - use where an Object already exists
15:38:08 <cait> - use preferrably
15:38:15 <cait> ß
15:38:16 <cait> ?
15:38:26 <khall> that sounds reasonable and good to me!
15:38:37 <tcohen> i think every medium sized project for improving Koha (new module) should be discussed with the core team so (without interfering) we agree on different alternative approaches
15:38:39 <cait> again, someone willing to draft?
15:38:52 <pianohacker> Joubu: are you referring to http://koha.1045719.n5.nabble.com/Koha-Object-td5848332.html ?
15:38:59 <Joubu> "core team" needs to be defined
15:39:06 <tcohen> arguments against an approach supporting the other will raise, win-win situation
15:39:11 <pianohacker> what Joubu said
15:39:13 <tcohen> koha-devel
15:39:14 <wahanui> well, koha-devel is most likely the best koha list there is (Hey, Im a bot, im biased) and can be found at http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
15:39:31 <Joubu> yep
15:39:44 <pianohacker> Joubu: and http://koha.1045719.n5.nabble.com/Koha-and-DBIC-td5811156.html ?
15:39:45 <tcohen> the worse situation we've seen is starvation
15:39:52 <khall> cait: I think we can vote on what you just wrote
15:39:56 <khall> it's simple enough
15:39:57 <tcohen> no one answers, deve does what he wants
15:39:58 <Joubu> pianohacker: yep
15:40:00 <tcohen> it's the RM call
15:40:05 <tcohen> period
15:40:27 <cait> i'd like something a bit more polished in full sentences for the wiki:)
15:40:38 <cait> that someone will udnerstand outside of this meeting
15:40:39 <khall> cait: ok, I'll get something written right now
15:40:53 <cait> and with a clear escape clause - as the proof might be in the code... i don't know
15:40:58 <cait> thx
15:41:03 <cait> moving on for now
15:41:05 <cait> the grandfather clause
15:41:24 <marcelr> grandfather is implicit for more rules
15:41:31 <marcelr> probably
15:41:37 <cait> my suggestion woudl be: we should state the date and the meeting in the guidelines added clearly. Then we can say, code submitted before guidelines with a clear 'start from' date is still ok
15:42:05 <Joubu> and with "feel free to provide an alternative if you are not convinced with this one" clause? :)
15:42:12 <cait> or 'grandfather rules stated below applies' or whatever
15:42:41 <pianohacker> cait: agreed
15:43:28 <tcohen> vote?
15:43:28 <wahanui> vote is probably going to the list regardless of what we decide
15:43:45 <marcelr> ... for exceptions to this rule we need convincing arguments for using or skipping DBIx/Koha::Object
15:43:57 <cait> formal vote or +1?
15:45:22 <cait> i'd like to stick with +1 actually, becuase it's a littler faster today
15:45:44 <cait> but again, i need someone to write it up - the grandfather rule
15:45:53 <khall> cait: will do
15:45:57 <cait> thx
15:46:26 <cait> moving on to next opic for now
15:46:32 <cait> [marcelr] New modules should be added if possible into the new Koha namespace (and not in C4).
15:46:36 <pianohacker> while it sounds like we're pushing the K::O discussion to the mailing list, should we vote on the grandfather rule here and now?
15:46:41 <cait> i think this is pretty clear
15:46:44 <pianohacker> sorry, just missed timing :)
15:46:58 <cait> pianohacker: will return to all the 'write up somethings' at the end
15:47:19 <barton> cait++
15:47:35 <khall> cait: I've got those writeups, should we do a quick review / vote?
15:47:44 <cait> yes, where?
15:47:55 <khall> cait: I was just going to paste them here
15:47:58 <khall> PERL16 - Modules in the Koha namespace should be object oriented when possible, using Koha::Object(s) as a preferred base.
15:47:59 <khall> PERL16.1 - If an Koha::Object already exists, use it instead of other methods of table CRUD.
15:48:20 <khall> XXX - Patches submitted before the introduction of a new rule may pass qa even if they do not meet the current coding guidline requirements of the discretion of the QA team member.
15:48:37 <khall> also, we'll want to put the date on new rules from now on
15:48:42 <khall> but that's implied
15:49:18 <pianohacker> cait: are there any patches old enough that we should do some wiki archeology to figure out the date-added of the existing rules?
15:49:33 <cait> pianohacker: i hope not, but probably will do so when the problem arises :)
15:49:42 <pianohacker> seems reasonable
15:50:07 <cait> any comments?
15:50:57 <cait> don't go quiet...
15:51:06 <khall> going once
15:51:25 <khall> twice
15:51:33 <cait> no comments?
15:51:47 <cait> ok, voting on PERL16/16.1 please
15:51:50 <khall> +1
15:51:54 <marcelr> +1
15:51:55 <pianohacker> +1
15:51:58 <kidclamp> +1
15:51:59 <barton> +1
15:52:06 <cc> -1
15:52:06 <indradg> +1
15:52:14 <Joubu> +1
15:52:17 <tcohen> +1
15:52:45 <bag> +1
15:53:06 <tajoli> +1
15:53:24 <cait> #agreed PERL16 - Modules in the Koha namespace should be object oriented when possible, using Koha::Object(s) as a preferred base.  PERL16.1 - If an Koha::Object already exists, use it instead of other methods of table CRUD.
15:53:50 <cait> for the XXX rule - at the discretion of the qa team member...
15:53:55 <khall> +1
15:54:03 <barton> +1
15:54:04 <cc> +1
15:54:07 <tajoli> +1
15:54:12 <cait> do we want to keep it that way? so i can say 'no'? :) heh
15:54:37 <marcelr> +1 for XXX :)
15:54:56 <Joubu> +1 #QA team member*s*
15:55:14 <marcelr> yeah there are more
15:55:19 <pianohacker> cait: my understanding of the wording (which I think should be slightly clarified) is that QA team members can choose to say _yes_, not no, on grandfathered patches
15:55:41 <pianohacker> khall: is that what you had in mind?
15:55:52 <bag> yes sounds about right khall
15:55:56 <khall> pianohacker is correct
15:56:04 <bag> :)
15:56:13 <cait> pianohacker: ah right - can say 'yes'
15:56:23 <Joubu> The 's' is important I think
15:56:36 <cait> qa team members?
15:56:40 <marcelr> Joubu: member is the member at hand
15:56:51 <marcelr> the one doing the qa
15:56:59 <cait> it's adifference indeed
15:57:10 <Joubu> I'd ask for another QA POV in this case, so I won't be alone
15:57:25 <pianohacker> that's probably a good thing to put in the guideline
15:57:29 <Joubu> What we did for the recovery pwd patchset for instance
15:57:40 <pianohacker> it's an existing habit, should be written down :)
15:57:47 <Joubu> it did not fit the requirements, *we* agreed to let it pass
15:57:48 <Joubu> anyway
15:57:56 <cait> hm what about
15:58:08 <marcelr> but is not a coding rule
15:58:20 <cait> Grandfather clause:  XXX - Patches submitted before the introduction of a new rule may pass QA even if they do not meet the current coding guideline requirements in agreement with the QA team.
15:58:25 <Joubu> ok, let's forget that, not important
15:58:49 <marcelr> whole team is not needed i guess
15:59:12 <cait> #agreed Grandfather clause:  XXX - Patches submitted before the introduction of a new rule may pass QA even if they do not meet the current coding guideline requirements after consulting with the QA team members. ?
15:59:17 <Joubu> imo, more than 1 is needed
15:59:23 <cait> i am nto sure how to phrase that there shoudl be a window for discussion
15:59:25 <marcelr> at discretion of QA team member (and possibly another QAer)
15:59:36 <cait> if noone chimes in... that woudl be ok
15:59:37 <marcelr> some things are trivial
16:00:12 <barton> I like that wording, cait.
16:00:31 <khall> I would also like to bring up bug 8753 for a quick discussion and vote. In that but I wrote two scripts, one for display, and the other for CRUD
16:00:32 <huginn`> 04Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 enhancement, P1 - high, ---, charles.farmer, Pushed to Master , Add forgot password link to OPAC
16:00:43 <cait> consulting would practically mean asking for opinions - if response is low... so it's up to those who work it out
16:00:52 <khall> the general consensus is that we shouldn't do this, so I think this should have a rule as well
16:01:09 <pianohacker> khall: do you mean bug 14610?
16:01:10 <huginn`> 04Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14610 enhancement, P5 - low, ---, kyle.m.hall, Signed Off , Add ability to place article requests in Koha
16:01:11 <cait> one thing after the other please
16:01:20 <cait> which is the preferred phrasing now? so i can add it to the minutes
16:01:21 <khall> cait: yes, you are correct ; )
16:01:50 <cait> quick please
16:01:51 <bag> yes cait is correct there - phew
16:01:52 <marcelr> khall: you cannot catch every exception in a rule (or you don't want to)
16:02:30 <khall> marcelr: I was failed qa for something not in the qa guidlines, I don't want that to happen to future developers or forgetful me ; )
16:02:31 <cait> i think what rangi said in argentina always applies... rules are good, but rules can be broken  (I choose to read broken as questioned :))
16:02:52 <cait> i thin we need to find somethin gin between
16:03:00 <khall> right, it's better to allow a rule to get broken than to fail someone who hasn't broken a rule
16:03:17 <cc> maybe less rules but more recommendations
16:03:18 <cait> anyway consulting, dicrestion or agreement?
16:03:19 <bag> good point
16:03:19 <wahanui> I know! The blade went right through that child!
16:03:26 <marcelr> i think it should be possible to fail although there is no rule
16:03:35 <marcelr> but you should explain of course
16:03:58 <marcelr> the rules are just tools :)
16:04:13 <cait> transparency is also key
16:04:30 <khall> yes, and a failure without a rule should generate discussion and a new rule
16:04:44 <cait> so can we finish please with the XXX rule?
16:04:48 <cait> so we can move on?
16:04:59 <khall> I thought the XXX rule was finished
16:05:00 <barton> cait: I'm ok with any of consulting, dicrestion or agreement.
16:05:09 <cait> there was discussion about the wording
16:05:10 <khall> I would propose PERLXX: CGI scripts that handle CRUD operatiosn ( Create, Rad, Update, Delete ) should all be handled in the same script
16:05:24 <khall> ok, let's table my recent discussion and finish XXX
16:05:31 <cait> thx
16:05:38 <cait> just tell me which, i will log and we can move on
16:06:11 <khall> the idea behind XXX is that even if a patch fails the current qa guidlines, a QA'er can still give it a pass if the rules it fails were made after the patch's submission
16:06:20 <cait> yes
16:06:39 <khall> should we revote, or are we just discussing the specific wording?
16:06:40 <cait> i think the question was if one qa'er or after asking other qa'ers for opinions
16:06:47 <cait> i think only the wording
16:06:50 <cait> if noone insists
16:07:03 <khall> well, we can always elevate it to the head of QA ; )
16:07:10 <cait> heh
16:07:22 <marcelr> only one rule
16:07:23 <cait> head of QA is mean i have heard... better not
16:07:40 <cait> i will log as suggested if there is no strong opinion
16:07:50 <cait> #agreed Grandfather clause:  XXX - Patches submitted before the introduction of a new rule may pass QA even if they do not meet the current coding guideline requirements of the discretion of the QA team member.
16:07:51 <cait> ok
16:07:52 <cait> next
16:08:06 <cait> Kyle suggested a rule about the handling of CRUD operations
16:08:16 <khall> PERLXX: CGI scripts that handle CRUD operatiosn ( Create, Rad, Update, Delete ) should all be handled in the same script when possible.
16:08:21 <cait> triggered by bug 14610
16:08:23 <huginn`> 04Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14610 enhancement, P5 - low, ---, kyle.m.hall, Signed Off , Add ability to place article requests in Koha
16:08:24 <khall> s/Rad/Read
16:08:37 <cait> any questions, comments?
16:08:56 <barton> that looks sensible to me.
16:08:57 <Joubu> I have failed it, so I am ok with the proposition
16:09:05 <cait> I also commented, so ok as well
16:09:08 <marcelr> sounds reasonable
16:09:11 <pianohacker> I think it's a good rule for future patches; a majority though not all of Koha follows this pattern
16:09:19 <cait> hah
16:09:24 <Joubu> I have rewritten the admin scripts to follow this pattern
16:09:37 <Joubu> using the same $op value would be better
16:09:56 <khall> we can include a script to point to as the best example
16:10:11 <Joubu> add_form, add_validate, list, delete_confirm, delete_confirmed
16:10:31 <Joubu> admin/cities is the smaller one I guess
16:10:32 <marcelr> afk
16:10:39 <cait> hm modify?
16:10:46 <Joubu> add_form
16:11:23 <Joubu> There is no need to have a different op, if an id is passed, you are modifying, otherwise you are adding
16:11:36 <khall> Joubu: I don't think we need to be *quite* that specific in the rule, but yes, that's what we are looking for.
16:12:13 <cait> ok, no general disagreement
16:12:25 <cait> i think?
16:12:34 <bag> no disagreement here
16:12:35 <Joubu> khall: if we don't specify, we will get different wording for the same action
16:12:46 <Joubu> but not important
16:13:01 <cait> i think adding an example would be good
16:13:08 <cait> as suggested
16:13:13 <cait> pointing to an existing file
16:13:20 <Joubu> so you can add admin/cities.pl
16:13:29 <khall> Joubu: I understand. We can work out a basic action list later, but I think the example should suffice for now
16:13:33 <cait> can we vote please?
16:13:34 <Joubu> if everybody had a look at it already
16:13:40 <pianohacker> no, though I think we should revisit Kyle's general concern about failing QA for rules not in the coding guidelines after we vote on this :)
16:13:47 <Joubu> khall: yep, agreed
16:13:50 <cc> We might want to show good examples - an example is more useful than a rule for someone learning the codebase
16:13:55 <barton> vote.
16:13:58 <cait> cc: totally agree
16:14:11 <khall> cc: also agreed
16:14:24 <cait> can we have +1 or -1 please?
16:14:29 <khall> +1
16:14:32 <barton> +1
16:14:33 <cait> +1
16:14:34 <Joubu> +1
16:14:35 <cc> +1
16:14:36 <pianohacker> cc: and it can help address Joubu's concern about consistency in the details without having to put them in stone
16:14:38 <pianohacker> +1
16:14:39 <kidclamp> +1
16:15:14 <bag> +1
16:15:26 <jajm> +1
16:15:47 <cait> #agreed PERLXX: CGI scripts that handle CRUD operatiosn ( Create, Rad, Update, Delete ) should all be handled in the same script when possible.
16:16:06 <cait> #info PERLXX shoudl link to admin/cities.pl for a good example
16:16:23 <cait> ok, any loose ends right now or can we move to next?
16:16:34 <barton> next.
16:16:36 <pianohacker> cait: what I mentioned above (:13t
16:16:46 <cait> ok, go for it
16:16:47 <Joubu> It's *my* good example, I don't know if everybody agrees with that
16:17:09 <khall> Joubu: I think everyone is happy with it
16:17:12 <cait> Joubu: noone disagreed so far... we will see if the topic reappears on next agenda ;)
16:17:28 <barton> Joubu++
16:18:07 <pianohacker> I'm not pushing for anything dramatic, but I think we should codify that, if a QA team member has concerns about coding style (as opposed to intended functionality not working correctly), they should be either in the coding guidelines or the patch should be used to introduce a new coding guideline in a meeting / koha/devel
16:18:09 <cait> I think failing without a coding guileline should actually be an 'in discussion' - it indicates a disagreement about something and it should lead to a discussion with more people / dev meeting
16:18:43 <cait> i see this is a defacto actually
16:18:58 <pianohacker> cait: I think that's somewhat the practice, but we should connect such things more strongly to the coding guidelines
16:19:05 <Joubu> pianohacker: if a dev wants to use a new pattern, the easier is to ask on koha-devel or here on #koha
16:19:05 <khall> I think you both agree. It'd like it to be dejure
16:19:13 <pianohacker> as it is, the guidelines are incomplete and somewhat unloved
16:19:18 <cait> khall: now i need the dictionary
16:19:35 <khall> cait: just means it should be a codified rule
16:19:53 <khall> de facto - rule in practice
16:20:05 <pianohacker> cait/Joubu: the main reason I'd like this (and khall as well, to my understanding), is that it would be much better to have up-to-date coding guidelines that reflect as much as possible of our current practices
16:20:06 <khall> de jure - rule codified in law ( i.e. coding guielines )
16:20:12 <cait> this is one of the thing sthat I am nt sure it needs to be a rule... it seems more like a right to me... people disagree, it should lead to discussion
16:20:25 <Joubu> it's impossible to list all we can do or not do :)
16:20:42 <cait> i'd just like to say that i thik we have followed that
16:20:52 <Joubu> but if you need to introduce a design change, ask other devs
16:20:55 <khall> Joubu: you are correct, but we should do out best to help out those devs that haven't been around for years
16:21:00 <pianohacker> so that code can be written to match those practices from the beginning
16:21:03 <pianohacker> and what khall said
16:21:05 <cait> and disagreeing with QA is fine (well... maybe not that TOO often ;) )
16:21:20 <pianohacker> Joubu: I agree not everything can be codified, but as much as is reasonable _should_ be :)
16:21:24 <cait> i am just not sure how that would read as a rule
16:22:03 <Joubu> pianohacker: yes, that's what we are doing for 90min :)
16:22:17 <cait> yeah... running out of time soon :)
16:22:20 <bag> :D
16:22:21 <bag> ha
16:22:36 <barton> aaand on that note... next topic?
16:22:39 <cait> i thnk maybe this can be both sides
16:23:00 <pianohacker> cait: I'll send a possible wording to the dev list after the meeting :)
16:23:06 <cait> if you introduce a design change - aks others... if you don't agree with a qa ruling... bring it to a dev meeting
16:23:09 <khall> thanks pianohacker!
16:23:18 <khall> QA court!
16:23:26 <cait> yeah... i'd like to avoid that a bit heh
16:23:33 <khall> jk ; )
16:23:43 <Joubu> I definitively agree to have a list of "best pratices" to fill when QAing and vote them on the next dev meeting
16:23:48 <cait> i mean it goes both ways - devs shoudl talk about what they do.. as should qa
16:24:09 <cait> Joubu: can you give a quick example?
16:24:17 <cait> pianohacker: ok i wil action
16:24:20 <Joubu> no, I don't have any in mind
16:24:36 <khall> the problem is often we get either radio silence or a split down the middle when we discuss things
16:24:37 <cait> #action pianohacker to suggest a possible guideline for handling disagreements
16:24:44 <Joubu> but I will try to think about that when failing qa
16:24:57 <cait> ok
16:25:10 <cait> we i think the namespace rule is already passed?
16:25:17 <khall> yes
16:25:26 <pianohacker> yeah, very very defacto, should be dejure
16:25:27 <khall> how about the update to JS4?
16:25:39 <oleonard_> Sorry, but what is JS4?
16:25:45 <pianohacker> oleonard_: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS4:_Avoid_joining_multiple_language_strings_with_other_variables
16:25:52 <pianohacker> khall: because of the new %s support?
16:25:58 <khall> right
16:26:01 <cait> sorry, help me
16:26:08 <cait> did we vote on 'new modules shoudl be in Koha::'?
16:26:26 <khall> oh, I mis-understood
16:26:28 <Joubu> It has been voted years ago, isn't it??
16:26:39 <cait> not in the coding guidelines as of today
16:26:41 <cait> an oversight
16:26:48 <khall> ok, quick vote on that?
16:26:51 <pianohacker> let's get that outta the way
16:27:02 <pianohacker> +1 to add 'new modules should be in Koha::'
16:27:09 <talljoy> +1
16:27:11 <cait> PERL XXX: New modules should be added to the Koha namespace as C4 is deprecated. ?
16:27:13 <khall> +1
16:27:16 <cait> +1
16:27:21 <cait> wording can be refined of course
16:27:30 <pianohacker> I agree with cait's proposed wording
16:27:31 <cc> +1
16:27:40 <khall> also agree
16:27:53 <barton> +1
16:28:05 <Joubu> +1 # yes please...
16:28:19 <cait> #agreed PERL XXX: New modules should be added to the Koha namespace as C4 is deprecated.
16:28:21 <cait> ok
16:28:24 <bag> +1
16:28:27 <Joubu> new subroutine as well actually...
16:28:30 <cait> bag, you are too slow :)
16:28:43 <talljoy> lol
16:28:54 <cait> Joubu: would you have tomove the wholemodule in this case? or just make a new one for the routine?
16:28:57 <pianohacker> Joubu: I think that's a bit too tricky to codify
16:29:14 <khall> cait: I'd like to table my sub-rule on that ( single argument stuff ) for another meeting so we can get though more important stuff this meeting ; )
16:29:25 <cait> we already voted on removing perl 18 / perl 19
16:29:32 <cait> i'd like to finish removals if ossible
16:29:37 <cait> then we will probably have to stop
16:29:42 <bag> (Ginny was distracting me) ;)
16:29:44 <khall> what's left for removals?
16:29:57 <Joubu> perl11 and perl12?
16:29:58 <cait> [kfischer] 'HTML5: Deprecation of the 'prog' and 'CCSR' OPAC themes.' Templats are long gone.
16:30:05 <cait> [kfischer] 'PERL11: No CVS - Development has moved from CVS to git.  Therefore the use of CVS keywords $Id$ and $Revision$ should be  discontinued.' Is this rule still needed?
16:30:08 <pianohacker> Joubu: things like adding new circ functionality as Koha:: modules without just calling back into a million C4 modules would be a pretty high bar
16:30:10 <cait> [kfischer] 'PERL12: VERSION [Deprecated as of start of 3.12 release  cycle] Each module should have a $VERSION variable to preserve  progressive levels of compatibility with dependent code.' A bit  confusing - should remaining VERSION variables be removed?
16:30:25 <cait> html5 - is easy i think
16:30:27 <pianohacker> I'm up for a vote to remove all of the above, any objections?
16:30:30 <cait> the code is no longer there .)
16:30:39 <Joubu> pianohacker: yes indeed there are exceptions :)
16:30:45 <cait> for the last one, just a question: shoudl re remove existing VERSION variables?
16:31:06 <cait> they appeared earlier today in discussion as not being readonly - but i am not sure what they are used for actually
16:31:13 <khall> easy enough to do
16:31:17 <Joubu> We never used them
16:31:34 <khall> I volunteer to remove them if we so wish
16:31:39 <pianohacker> would that break any plugin that put the version number in the use statement?
16:32:01 <khall> pianohacker: not plugins I'm aware of do that.
16:32:12 <khall> plugins can specify a *Koha* version though
16:32:15 <pianohacker> then I'd say kill 'em
16:32:16 <cait> I think the koha plugins check version differently?
16:32:22 <cc> we say run perlcritic at one point but it complains if VERSION is missing
16:32:28 <cait> oh
16:32:32 <pianohacker> cc: is that configurable?
16:32:55 <cait> hm haven't seen that in my qa script - possible we already did?
16:33:09 <Joubu> there is certainly a perlcritic rule to remove it
16:33:10 <cc> almost certainly -- (if you want to spend time configuring it)
16:34:02 <pianohacker> Joubu: are you comfortable volunteering to make perlcritic happy?
16:34:03 <cait> hm right now i wonder where i got my perlcritic file from... that it uses
16:34:14 <cait> let's postpone that one and vote on the other 2 for now?
16:34:17 <pianohacker> yar
16:34:27 <khall> sounds good
16:34:29 <cait> please vote on removal of html5 and perl12
16:34:32 <pianohacker> cait: well, I think we're in agreement about removing the rule
16:34:33 <cait> argh
16:34:33 <Joubu> Actually I don't understand, we have added some new module without the $VERSION var defined, and the tests passed
16:34:36 <cait> perl 11
16:34:39 <khall> +1
16:34:44 <cait> html5 and perl11
16:34:48 <pianohacker> but +1
16:34:52 <cc> +1
16:34:53 <tajoli> +1
16:34:59 <Joubu> +1
16:35:00 <cait> Joubu: yeah i think we need to investigate :)
16:35:04 <cait> +1
16:35:05 <barton> +1
16:35:08 <bag> +1
16:35:11 <talljoy> +1
16:35:37 <kidclamp> +1
16:35:50 <cait> #agreed to remove PERL11: No CVS - Development has moved from CVS to git. Therefore the  use of CVS keywords $Id$ and $Revision$ should be discontinued.
16:36:02 <cait> #agreed to remove HTML5: Deprecation of the 'prog' and 'CCSR' OPAC themes.' Templats are long gone
16:36:21 <cait> ok
16:36:29 <barton> *templates
16:36:31 <cait> who is going to amek the agreed to changes?
16:36:40 <cait> barton: toolate :)
16:36:55 <cait> I can try to - but woudl encourage people to check the wiki changes
16:37:02 <cait> i can send an email to the list
16:37:18 <cait> once done
16:37:41 <cait> ok?
16:37:43 <khall> cait: let me know when you've done it and I can review it
16:37:46 <barton> np.
16:37:53 <tajoli> ok
16:38:08 <cait> #action cait to apply agreed to changes on the wiki
16:38:21 <cait> that way... if i messed up the minutes i will have to sort it out :)
16:38:30 <cait> bag: next meeting?
16:38:31 <wahanui> it has been said that next meeting is December 5th 18 UTC
16:38:42 <cait> forget next meeting
16:38:43 <wahanui> cait: I forgot next meeting
16:39:06 <bag> looking
16:39:27 <pianohacker> should we go for sooner than later, as there's plenty we couldn't get to?
16:39:35 <bag> 2nd of March ?
16:39:37 <khall> would be nice if we could do bi-monthly dev meetings ( 2 a month ). Is that too often ?
16:39:48 <khall> I think sooner would be better
18:18:49 <gmcharlt> @quote random
18:18:49 <huginn> gmcharlt: Quote #240: "<wizzyrea> we will have no hazing of RM's" (added by gmcharlt at 10:22 PM, April 05, 2013)
18:23:05 <pianohacker> bug 12321
18:23:06 <huginn> 04Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12321 normal, P5 - low, ---, gmcharlt, NEW , indicators not editable after linking authority
18:23:09 <pianohacker> cool
18:23:12 <pianohacker> gmcharlt++
18:42:16 <pianohacker> gmcharlt: any idea why it died?
18:42:38 <gmcharlt> pianohacker: Linode in Atlanta sufferred a routing snafu
18:43:20 <pianohacker> ah kk
18:46:17 <oleonard> My error logs these days are all "CGI::param called in list context from package..." Makes it hard to see the real issues.
18:47:41 <pianohacker> do we even have a bug for that yet?
18:49:58 <cait> oh you got huginn breathing again
18:50:05 <cait> gmcharlt: anyhtng we could do about the meeting logs?
18:52:05 <oleonard> pianohacker: Bug 14121 similar?
18:52:06 <huginn> 04Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14121 minor, P5 - low, ---, mtompset, RESOLVED FIXED, Silence warnings t/db_dependent/Auth_with_cas.t
18:56:21 <pianohacker> oleonard: yeah, that's the same error
19:00:40 <gaetan_B> bye
19:06:06 <oleonard> I imagine there exists a project where the newer bug always gets marked as the duplicate even if it has patches on it.
19:08:16 <cait> maybe it's a language thing?
19:08:36 <cait> i haven't read all of today's bug mail yet - guess i will find it sooner or later
20:45:54 <gmcharlt> hmm
20:45:58 <gmcharlt> #endmeeting
20:46:29 <cait> hm, he forgot :(
20:47:10 <gmcharlt> hmm, could you try it, cait? since you started the meeting?
20:47:18 <cait> sure
20:47:20 <cait> #endmeeting