Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sierra: Add translation prefix support for ILS messages. #3140

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

EreMaijala
Copy link
Contributor

Very much alike the one in Alma.

@EreMaijala EreMaijala requested a review from demiankatz October 4, 2023 10:18
Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, apart from a minor complaint about the comments.

; not used, but a prefix may be used to distinguish the codes from any other
; translations (or other libraries). To use a simple prefix in the default text
; domain:
; translationPrefix = "sierra_"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments are a little confusing here; you have text explaining the first example, but no text explaining the second example... and then the "actual" commented-out setting is identical to the second example. How would you feel about eliminating the two example lines, and changing the last sentence to something like the following?

You can use a simple prefix in the default domain (e.g. "sierra_") or a custom text domain (e.g. "ILSMessages::").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was just copied from Alma.ini, but I'll revise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be better now.

Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @EreMaijala!

@demiankatz demiankatz merged commit f7a9fc7 into vufind-org:dev Oct 4, 2023
6 checks passed
@demiankatz demiankatz deleted the dev-sierra-translation-prefix branch October 4, 2023 11:29
EreMaijala added a commit to NatLibFi/NDL-VuFind2 that referenced this pull request Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants