|
Description
Tomás Cohen Arazi (tcohen)
2018-08-16 09:54:08 UTC
Created attachment 77880 [details] [review] Bug 21233: Unit tests This patch adds tests for the required password-related exceptions. The tests verify the stringified version of the exceptions. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Exceptions not implemented, tests fail! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 77881 [details] [review] Bug 21233: Add Koha::Exceptions::Password This patch adds some exceptions that we need. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off! :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 77914 [details] [review] Bug 21233: Unit tests This patch adds tests for the required password-related exceptions. The tests verify the stringified version of the exceptions. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Exceptions not implemented, tests fail! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 77915 [details] [review] Bug 21233: Add Koha::Exceptions::Password This patch adds some exceptions that we need. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off! :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> FAIL Koha/Exceptions/Password.pm
FAIL pod coverage
POD is missing for 'description'
This might be a qa tools bug?
podchecker does not warn on this module.
Reversing patch order.. Just a language thing on TrailingWhitespaces I would say we have trailing spaces or trailing whitespace where whitespace is \s* in regex terms (catching all spaces, tabs, newlines etc.). No plural form needed. From https://english.stackexchange.com/questions/25368/what-is-the-plural-form-of-whitespace: The word "whitespace" is usually uncountable. If you need it to be countable, I would suggest saying "whitespace characters". For some other report: git grep -l [wW]hitespaces :) Created attachment 78116 [details] [review] Bug 21233: Add Koha::Exceptions::Password This patch adds some exceptions that we need. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => SUCCESS: Tests pass! - Sign off! :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 78117 [details] [review] Bug 21233: Unit tests This patch adds tests for the required password-related exceptions. The tests verify the stringified version of the exceptions. To test: - Apply this patch - Run: $ kshell k$ prove t/Koha/Exceptions.t => FAIL: Exceptions not implemented, tests fail! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 78118 [details] [review] Bug 21233: (QA follow-up) We do not like the plural form of whitespace The word "whitespace" is usually uncountable. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Marcel de Rooy from comment #5) > FAIL Koha/Exceptions/Password.pm > FAIL pod coverage > POD is missing for 'description' > > This might be a qa tools bug? > podchecker does not warn on this module. That's a false positive. Yes. (In reply to Marcel de Rooy from comment #7) > Just a language thing on TrailingWhitespaces > I would say we have trailing spaces or trailing whitespace where whitespace > is \s* in regex terms (catching all spaces, tabs, newlines etc.). No plural > form needed. > > From > https://english.stackexchange.com/questions/25368/what-is-the-plural-form-of- > whitespace: > The word "whitespace" is usually uncountable. If you need it to be > countable, I would suggest saying "whitespace characters". I'd say we should even get rid of 'Trailing', because this is then used in a test for whitespace chars at the begining too, right? (In reply to Tomás Cohen Arazi from comment #13) > (In reply to Marcel de Rooy from comment #7) > > Just a language thing on TrailingWhitespaces > > I would say we have trailing spaces or trailing whitespace where whitespace > > is \s* in regex terms (catching all spaces, tabs, newlines etc.). No plural > > form needed. > > > > From > > https://english.stackexchange.com/questions/25368/what-is-the-plural-form-of- > > whitespace: > > The word "whitespace" is usually uncountable. If you need it to be > > countable, I would suggest saying "whitespace characters". > > I'd say we should even get rid of 'Trailing', because this is then used in a > test for whitespace chars at the begining too, right? Too late ? Created attachment 78721 [details] [review] Bug 21233: (QA follow-up) It is not only trailing whitespace characters Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Marcel de Rooy from comment #14) > (In reply to Tomás Cohen Arazi from comment #13) > > (In reply to Marcel de Rooy from comment #7) > > > Just a language thing on TrailingWhitespaces > > > I would say we have trailing spaces or trailing whitespace where whitespace > > > is \s* in regex terms (catching all spaces, tabs, newlines etc.). No plural > > > form needed. > > > > > > From > > > https://english.stackexchange.com/questions/25368/what-is-the-plural-form-of- > > > whitespace: > > > The word "whitespace" is usually uncountable. If you need it to be > > > countable, I would suggest saying "whitespace characters". > > > > I'd say we should even get rid of 'Trailing', because this is then used in a > > test for whitespace chars at the begining too, right? > > Too late ? :-D Awesome work all! Pushed to master for 18.11 Created attachment 78836 [details] [review] Bug 21233: (follow-up) POD fix Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Tomás Cohen Arazi from comment #18) > Created attachment 78836 [details] [review] [review] > Bug 21233: (follow-up) POD fix > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Follow up pushed to master Enhancement, will not be backported to 18.05.x series. |