https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16149

Jonathan Druart <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]
                   |                            |ommunity.org
             Status|Signed Off                  |Failed QA

--- Comment #16 from Jonathan Druart <[email protected]> 
---
1.
 FAIL   C4/Reports/Guided.pm
   FAIL   valid
                "my" variable $report masks earlier declaration in same scope 

 FAIL   misc/cronjobs/patron_emailer.pl
   FAIL   spelling
                 ommited  ==> omitted
                 sucesses  ==> successes
   FAIL   valid
                "my" variable $report masks earlier declaration in same scope 

* Commit title does not start with 'Bug XXXXX: ' - 2380dc8
* Commit title does not start with 'Bug XXXXX: ' - 579e02c

2. I think we display something more useful than the error codes

3. Please correct indentation

4. Reading the tests:
is( $result[0]{NO_EMAIL_COL}, 2, "Warning only for patron with no email");
Why 2?

5. The cronjob script has too many use statements

6. I do not think the subroutine should take "commit", maybe it should return
the notice instead?
Same for verbose actually.

7. Subroutine takes "branch" whereas the script pass "branchcode" (!)

8. POD says "--report Specify a saved SQL report in the Koha system to user for
the emails."
It's actually the *id* (saved_sql.id), maybe it should be more explicit

9. --email, should not we use Koha::Patron->notice_email_address and remove
this option?

10.  --from                       specified email for 'from' address, report
column 'from' used if not specified

Should not we use branches.branchemail instead?

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
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/

Reply via email to