https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20478
Katrin Fischer <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] Status|NEW |Needs Signoff --- Comment #4 from Katrin Fischer <[email protected]> --- Hi Andreas, thx for your patches! Some things: 1) Once you submitted your patches it's important to switch the status to 'Needs Signoff' in order to get the attention of the testers, otherwise patches can get easily lost in the saved searches. 2) The QA test tools can help you to find problems with your code easily. For your patches the report is: FAIL misc/cronjobs/advance_notices.pl OK critic FAIL forbidden patterns forbidden pattern: tab char (line 367) forbidden pattern: tab char (line 479) forbidden pattern: tab char (line 366) forbidden pattern: tab char (line 374) forbidden pattern: tab char (line 357) forbidden pattern: tab char (line 373) forbidden pattern: tab char (line 372) forbidden pattern: tab char (line 348) forbidden pattern: tab char (line 356) forbidden pattern: tab char (line 350) forbidden pattern: tab char (line 368) forbidden pattern: tab char (line 358) forbidden pattern: tab char (line 347) forbidden pattern: tab char (line 349) forbidden pattern: tab char (line 355) forbidden pattern: tab char (line 365) forbidden pattern: tab char (line 353) forbidden pattern: tab char (line 369) forbidden pattern: tab char (line 371) forbidden pattern: tab char (line 352) forbidden pattern: tab char (line 354) forbidden pattern: tab char (line 364) forbidden pattern: tab char (line 351) forbidden pattern: tab char (line 370) FAIL t/db_dependent/cronjobs/advance_notices_digest.t FAIL critic Expression form of "eval" at line 167, column 1. See page 161 of PBP. Bareword file handle opened at line 158, column 1. See pages 202,204 of PBP. FAIL forbidden patterns forbidden pattern: tab char (line 67) 3) This can all be fixed, but I see another problem here: libraries are differently organized and we support a lot of different use cases with Koha. Some libraries want Koha to only send one email instead of many, others like yours don't want this behaviour. So we need to find a compromise here and a way to make this configurable. I think a system preference could work here as I don't see how to add it to the notices configuration in a good way. 4) We usually don't test .pl files yet... but not opposed to more tsts :) Tests are failing for me right now: t/db_dependent/cronjobs/advance_notices_digest.t .. 1/5 # Failed test 'There are two messages in the queue' # at t/db_dependent/cronjobs/advance_notices_digest.t line 175. # got: '0' # expected: '2' # Looks like you planned 5 tests but ran 1. # Looks like you failed 1 test of 1 run. t/db_dependent/cronjobs/advance_notices_digest.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 5/5 subtests 5) If you want, you could also add an individual Sponsord-By line for each sponsoring library. It depends on how you want the output in the release notes later on. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
