Created attachment 46955 [details] [review] 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.
Created attachment 46958 [details] [review] Bug 15618: Typos fixed in circulation.pref
Am still working on this - not ready to sign off.
Created attachment 47063 [details] [review] Bug 15618: Typos fixed in enhanced_content.pref
Created attachment 47065 [details] [review] Bug 15618: Typos fixed in logs.pref
Created attachment 47066 [details] [review] Bug 15618: Typos fixed in opac.pref
Created attachment 47070 [details] [review] Bug 15618: Typos fixed in searching.pref
Created attachment 47072 [details] [review] Bug 15618: Typos fixed in staff_client.pref
This is ready for sign-off. Look through the patches to confirm everything is correct and check I didn't miss anything.
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>
(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).
(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"
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>
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>
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>
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>
Created attachment 47147 [details] [review] Bug 15618: Typos fixed in enhanced_content.pref This page should work properly now!
Created attachment 47148 [details] [review] Bug 15618: Typos fixed in circulation.pref
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 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.
> @@ +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.
(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.
Created attachment 47282 [details] Order of Syndetics sysprefs
(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.
Gave a review, made a grammar, spelling, and typo check. I did not notice anything weird or misplaced.
Perhaps this one would be better off split into separate reports, or at least squashed down into a single patch
It might be nice to rescue these!