Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add translator for Public Record Office Victoria #3233
Add translator for Public Record Office Victoria #3233
Changes from 2 commits
5bf4054
327f94f
7b3e64b
86b46fa
ceba7d8
2fdc774
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AbeJellinek -- could you weigh in on this. This will work (i.e. write the string to Extra), but I'm not sure how comfortable we are using this behavior in translators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going on what has been recommended in the forums for date ranges. Eg: https://forums.zotero.org/discussion/63073/date-year-range and https://forums.zotero.org/discussion/105949/date-range
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will work everywhere? But it's safer (and more indicative of the final result) to do
Even though that is definitely more verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And we should use that for
archive-place
as well. ("Archive Place: "
should work.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I was referring to using item.issued to putting this into Extra -- the approach to date ranges using
Extra
is definitely right.I kind of like the less verbose version using
item.issued
-- is there actually a reason not to do this? will also be much faster to convert if we do get a better date field.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I guess not, assuming that actually works everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to, although it translates
archive-place
toArchive-place
, which is a little uglier than if we added it to Extra manually asArchive Place
. That's arguably a client bug, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed all the
extra
additions to use the more verbose form. I also changedseries
toArchive Collection
to be in line with CSL.