Bug 15618 - Small typos in system preferences
Summary: Small typos in system preferences
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-19 22:51 UTC by Aleisha Amohia
Modified: 2022-12-06 02:50 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: String patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 15618: Typos fixed in admin.pref (3.08 KB, patch)
2016-01-19 22:52 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in circulation.pref (4.31 KB, patch)
2016-01-19 23:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in enhanced_content.pref (2.76 KB, patch)
2016-01-20 23:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in logs.pref (1.30 KB, patch)
2016-01-20 23:31 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in opac.pref (1.36 KB, patch)
2016-01-20 23:44 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in searching.pref (1.88 KB, patch)
2016-01-21 00:36 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in staff_client.pref (2.06 KB, patch)
2016-01-21 00:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15618: Typos fixed in admin.pref (3.18 KB, patch)
2016-01-21 13:34 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15618: Typos fixed in logs.pref (1.42 KB, patch)
2016-01-21 14:14 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15618: Typos fixed in opac.pref (1.49 KB, patch)
2016-01-21 14:19 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15618: Typos fixed in searching.pref (2.00 KB, patch)
2016-01-21 14:22 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15618: Typos fixed in staff_client.pref (2.19 KB, patch)
2016-01-21 14:25 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 15618: Typos fixed in enhanced_content.pref (2.82 KB, patch)
2016-01-21 21:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 15618: Typos fixed in circulation.pref (4.31 KB, patch)
2016-01-21 21:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Order of Syndetics sysprefs (36.38 KB, image/png)
2016-01-25 22:07 UTC, Aleisha Amohia
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2016-01-19 22:51:28 UTC

    
Comment 1 Aleisha Amohia 2016-01-19 22:52:46 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2016-01-19 23:09:53 UTC Comment hidden (obsolete)
Comment 3 Aleisha Amohia 2016-01-19 23:10:15 UTC
Am still working on this - not ready to sign off.
Comment 4 Aleisha Amohia 2016-01-20 23:17:25 UTC Comment hidden (obsolete)
Comment 5 Aleisha Amohia 2016-01-20 23:31:02 UTC Comment hidden (obsolete)
Comment 6 Aleisha Amohia 2016-01-20 23:44:21 UTC Comment hidden (obsolete)
Comment 7 Aleisha Amohia 2016-01-21 00:36:18 UTC Comment hidden (obsolete)
Comment 8 Aleisha Amohia 2016-01-21 00:53:31 UTC Comment hidden (obsolete)
Comment 9 Aleisha Amohia 2016-01-21 01:11:55 UTC
This is ready for sign-off. 
Look through the patches to confirm everything is correct and check I didn't miss anything.
Comment 10 Owen Leonard 2016-01-21 13:34:09 UTC
Created attachment 47107 [details] [review]
[SIGNED-OFF] Bug 15618: Typos fixed in admin.pref

To test:
Confirm all typos under the Administration tab are fixed and that I
haven't missed any.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 11 Owen Leonard 2016-01-21 13:58:24 UTC
(In reply to Aleisha Amohia from comment #4)
> Created attachment 47063 [details] [review] [review]
> Bug 15618: Typos fixed in enhanced_content.pref

This one doesn't work for me.

Syck parser (line 58, column 15): syntax error at /usr/lib/x86_64-linux-gnu/perl5/5.20/YAML/Syck.pm line 75.

I think it's the use of a colon followed by a space. Since I don't know offhand what the rules are for this kind of data structure I look at other instances where a colon is used and I see that if you wrap the string in single quotes that works (see for instance line 70, 'Baker and Taylor "My Library Bookstore"...). Simply removing the space works too.

I think it would also be nice to format it like this:

- 'The URL for the Babeltheque javascript file (eg. http://www.babeltheque.com/bw_XX.js):'
- pref: Babeltheque_url_js


This seems to be a more common format (Description: URL).
Comment 12 Owen Leonard 2016-01-21 14:11:43 UTC
(In reply to Aleisha Amohia from comment #2)
> Created attachment 46958 [details] [review] [review]
> Bug 15618: Typos fixed in circulation.pref

This one gives me an error too:

Syck parser (line 283, column -1): syntax error at /usr/lib/x86_64-linux-gnu/perl5/5.20/YAML/Syck.pm line 75.

I think it's bad single quote/double quote combinations on lines 281 and 428. I think you need to use single quotes if the whole string is wrapped in double quotes and vice versa.

Also:

"checkouts of items which are reserved to someone else. If allowed do not generate RESERVE_WAITING and RESERVED warning. This allows self checkouts for those items."

We try to standardize on the word "hold" instead of "reserve," so I suggest:

"checkouts of items which are on hold for someone else. If allowed do not generate RESERVE_WAITING and RESERVED warning. This allows self checkouts for those items."

"Reserves" at this time is only used when referring to Course Reserves.

If we are making terminology changes to this file, it would be good to catch the instances of the word "borrower" which should be replaced with "patron." Also look at places where the word "issue" is used in descriptions to mean "check out." For instance:

"update a bibliographic record's total issues count whenever an item is issued"

Using the default English translation's terminology standard that should be:

"update a bibliographic record's total checkouts count whenever an item is checked out"
Comment 13 Owen Leonard 2016-01-21 14:14:55 UTC
Created attachment 47118 [details] [review]
[SIGNED-OFF] Bug 15618: Typos fixed in logs.pref

Typos fixed in logs.pref

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Owen Leonard 2016-01-21 14:19:35 UTC
Created attachment 47119 [details] [review]
[SIGNED-OFF] Bug 15618: Typos fixed in opac.pref

Typos fixed in opac.pref

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 15 Owen Leonard 2016-01-21 14:22:27 UTC
Created attachment 47120 [details] [review]
[SIGNED-OFF] Bug 15618: Typos fixed in searching.pref

Typos fixed in searching.pref

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 16 Owen Leonard 2016-01-21 14:25:55 UTC
Created attachment 47121 [details] [review]
[SIGNED-OFF] Bug 15618: Typos fixed in staff_client.pref

Typos fixed in staff_client.pref

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 17 Aleisha Amohia 2016-01-21 21:07:30 UTC
Created attachment 47147 [details] [review]
Bug 15618: Typos fixed in enhanced_content.pref

This page should work properly now!
Comment 18 Aleisha Amohia 2016-01-21 21:17:58 UTC
Created attachment 47148 [details] [review]
Bug 15618: Typos fixed in circulation.pref
Comment 19 Mark Tompsett 2016-01-25 04:55:03 UTC
Comment on attachment 47147 [details] [review]
Bug 15618: Typos fixed in enhanced_content.pref

Review of attachment 47147 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.pref
@@ +182,4 @@
>                choices:
>                    yes: Use
>                    no: "Don't use"
> +            - content from Syndetics. Note that this requires that you have signed up for the service and entered in your client code above.

Line 187 and 188 visible in this patch make it look like below is correct.
Comment 20 Mark Tompsett 2016-01-25 05:03:33 UTC
Comment on attachment 47148 [details] [review]
Bug 15618: Typos fixed in circulation.pref

Review of attachment 47148 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
@@ +161,4 @@
>                choices:
>                    yes: Allow
>                    no: "Don't allow"
> +            - checkouts of items which are on hold to someone else. If allowed do not generate RESERVE_WAITING and RESERVED warning. This allows self checkouts for those items.

"reserved to" is wrong, but I think this should be either "reserved by" (less change and aligns with RESERVE message portions) or "which are on hold for".

@@ +278,4 @@
>          -
>              - "Restrict patrons with the following target audience values from checking out inappropriate materials:"
>              - pref: AgeRestrictionMarker
> +            - "E.g. Enter target audience keyword(s) split by | (pipe) FSK|PEGI|Age| (No white space near |). Be sure to map agerestriction in Koha to MARC mapping (e.g. 521$a). A MARC field value of FSK 12 or PEGI 12 would mean: 'Borrower must be 12 years old'. Leave empty to not apply an age restriction."

I was noticing 856u in another patch on this bug report. Are we doing 856u or 856$u? We have 521$a here. Should it be 521a?

@@ -365,5 @@
>                choices:
> -                  confirm: "require confirmation"
> -                  alert: "display a message"
> -                  nothing : "do nothing"
> -            - .

I like this '.' removal and placing at the end of the strings.

@@ +523,5 @@
>              - pref: emailLibrarianWhenHoldIsPlaced
>                choices:
> +                  yes: Do
> +                  no:  "Don't"
> +            - "send an email to the Koha administrator email address whenever a hold request is placed."

"Send" "Don't send"? I don't see why this change is necessary.
Comment 21 Katrin Fischer 2016-01-25 07:20:19 UTC
> @@ +278,4 @@
> >          -
> >              - "Restrict patrons with the following target audience values from checking out inappropriate materials:"
> >              - pref: AgeRestrictionMarker
> > +            - "E.g. Enter target audience keyword(s) split by | (pipe) FSK|PEGI|Age| (No white space near |). Be sure to map agerestriction in Koha to MARC mapping (e.g. 521$a). A MARC field value of FSK 12 or PEGI 12 would mean: 'Borrower must be 12 years old'. Leave empty to not apply an age restriction."
> 
> I was noticing 856u in another patch on this bug report. Are we doing 856u
> or 856$u? We have 521$a here. Should it be 521a?

I think 521$u makes sense, if we have other cases without $ we could fix them separately. I think some options require you to enter without $, so have to be careful fixing.

> 
> @@ -365,5 @@
> >                choices:
> > -                  confirm: "require confirmation"
> > -                  alert: "display a message"
> > -                  nothing : "do nothing"
> > -            - .
> 
> I like this '.' removal and placing at the end of the strings.

The extra . at the end creates a separate string in the translation files and is a little advantage for translations. Because it's not always easy to built a grammatically correct sentence that has the options at the end, this extra string for the . allows to split the sentence up and have some words after the option list - not sure if I used it here, but I did in some other cases.

> 
> @@ +523,5 @@
> >              - pref: emailLibrarianWhenHoldIsPlaced
> >                choices:
> > +                  yes: Do
> > +                  no:  "Don't"
> > +            - "send an email to the Koha administrator email address whenever a hold request is placed."
> 
> "Send" "Don't send"? I don't see why this change is necessary.

I think the change makes it a bit easier to read.
Comment 22 Aleisha Amohia 2016-01-25 22:06:31 UTC
(In reply to M. Tompsett from comment #19)
> Comment on attachment 47147 [details] [review] [review]
> Bug 15618: Typos fixed in enhanced_content.pref
> 
> Review of attachment 47147 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> :::
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/enhanced_content.
> pref
> @@ +182,4 @@
> >                choices:
> >                    yes: Use
> >                    no: "Don't use"
> > +            - content from Syndetics. Note that this requires that you have signed up for the service and entered in your client code above.
> 
> Line 187 and 188 visible in this patch make it look like below is correct.

I will attach a screenshot that shows the order of the sysprefs for me and the reason I changed it to above.
Comment 23 Aleisha Amohia 2016-01-25 22:07:02 UTC
Created attachment 47282 [details]
Order of Syndetics sysprefs
Comment 24 Aleisha Amohia 2016-01-25 22:14:11 UTC
(In reply to Katrin Fischer from comment #21)
> > @@ +278,4 @@
> > >          -
> > >              - "Restrict patrons with the following target audience values from checking out inappropriate materials:"
> > >              - pref: AgeRestrictionMarker
> > > +            - "E.g. Enter target audience keyword(s) split by | (pipe) FSK|PEGI|Age| (No white space near |). Be sure to map agerestriction in Koha to MARC mapping (e.g. 521$a). A MARC field value of FSK 12 or PEGI 12 would mean: 'Borrower must be 12 years old'. Leave empty to not apply an age restriction."
> > 
> > I was noticing 856u in another patch on this bug report. Are we doing 856u
> > or 856$u? We have 521$a here. Should it be 521a?
> 
> I think 521$u makes sense, if we have other cases without $ we could fix
> them separately. I think some options require you to enter without $, so
> have to be careful fixing.
> 
I can find which patch you said had the 856u and change that to 856$u if that is the preferred format.

> > 
> > @@ -365,5 @@
> > >                choices:
> > > -                  confirm: "require confirmation"
> > > -                  alert: "display a message"
> > > -                  nothing : "do nothing"
> > > -            - .
> > 
> > I like this '.' removal and placing at the end of the strings.
> 
> The extra . at the end creates a separate string in the translation files
> and is a little advantage for translations. Because it's not always easy to
> built a grammatically correct sentence that has the options at the end, this
> extra string for the . allows to split the sentence up and have some words
> after the option list - not sure if I used it here, but I did in some other
> cases.
> 
Sorry I'm not sure if I understand, so you'd like the full stop in a separate string like it was before I changed it? To me it just looked strange to have a full stop lingering on the end, but if it is better for translation to have it the way it was before then it'll need to be changed in more places than just my patches.

> > 
> > @@ +523,5 @@
> > >              - pref: emailLibrarianWhenHoldIsPlaced
> > >                choices:
> > > +                  yes: Do
> > > +                  no:  "Don't"
> > > +            - "send an email to the Koha administrator email address whenever a hold request is placed."
> > 
> > "Send" "Don't send"? I don't see why this change is necessary.
> 
> I think the change makes it a bit easier to read.
Before it said 'Enable / Don't enable sending an email' which I thought could be made simpler with 'Do / Don't send'. However 'Send / Don't send' is basically a shortcut of that too so I can make that change if you'd both like.
Comment 25 Ian Hank 2016-01-28 18:56:48 UTC
Gave a review, made a grammar, spelling, and typo check. I did not notice anything weird or misplaced.
Comment 26 David Cook 2022-12-06 02:50:08 UTC
Perhaps this one would be better off split into separate reports, or at least squashed down into a single patch