-
Notifications
You must be signed in to change notification settings - Fork 18
Update 4130.xml #1258
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
base: master
Are you sure you want to change the base?
Update 4130.xml #1258
Conversation
added Syriac placeNames and related bibliographical sources
wlpotter
left a comment
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.
Thanks @mgbilby , overall this looks great! I've added a few comments for places to look at to make changes, but mostly small stuff. Well done!
data/places/tei/4130.xml
Outdated
| <revisionDesc status="draft"> | ||
| <change who="http://syriaca.org/documentation/editors.xml#wpotter" when="2025-06-18-05:00">CHANGED: Aligned Syriac World records with new places data model and schema</change> | ||
| <change who="http://syriaca.org/documentation/editors.xml#wpotter" when="2020-06-19-05:00">CREATED: place</change> | ||
| <change who="http://syriaca.org/documentation/editors.xml" when="2025-07-30-12:00">CHANGED: added Syriac placeNames and related bibliographical sources</change> |
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.
This is great! You can add your specific editor ID you shared with us: mgbilby, so the @who would be http://syriaca.org/documentation/editors.xml#mgbilby
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.
Note that this will give a validation error since we'll need to add you to that document, but you can ignore that error for now. We'll add participants to that document over the next week or so
data/places/tei/4130.xml
Outdated
| <listPlace> | ||
| <place ana="http://syriaca.org/taxonomy/regions" type="region"> | ||
| <placeName xml:id="name4130-1" srophe:tags="#syriaca-headword" xml:lang="en" source="#bib4130-1 #bib4130-2">Judean Desert</placeName> | ||
| <placeName resp="http://syriaca.org" xml:id="name4130-1" srophe:tags="#syriaca-headword" xml:lang="en" source="#bib4130-1 #bib4130-2">Judean Desert</placeName> |
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.
If there is a @source attribute, it does not need a @resp attribute
data/places/tei/4130.xml
Outdated
| <citedRange unit="p">88-104</citedRange> | ||
| </bibl> | ||
| <bibl xml:id="bib4130-4"> | ||
| <title xml:lang="en" level="a">Gospel of Matthew</title> |
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 need to add the title, editor, etc. elements here. They are permitted, but will be overwritten by the information in the CBSS bibliographic record on the website. More just for time saving you can leave these off
data/places/tei/4130.xml
Outdated
| <editor>G. H. Gwilliam</editor> | ||
| <editor>J. Pinkerton</editor> | ||
| <editor>John Gwynn</editor> | ||
| <ptr target="https://syriaccorpus.org/100#Head-id.3"/> |
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.
We want to limit the bibl/ptr/@target to just be CBSS URIs, but this is a great idea for inclusion! We need to add this to our encoding Guidelines, but the way we'd do this would be to use the Digital Syriac Corpus' overall URI (http://syriaca.org/cbss/M9AGRZUE). Then you can add a cited range like below to add a hyperlink to a specific section of the text on the Corpus:
<citedRange unit="entry" target="https://syriaccorpus.org/100#Head-id.3">https://syriaccorpus.org/100#Head-id.3</citedRange>
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.
We hope eventually to have specific entries in CBSS for the texts on the Corpus, so you can cite each text specifically. But we haven't yet implemented that, so this is a valid workaround
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.
Excellent. Thank you for constructing the cited range string for me to include. I like that solution.
made changes recommended by https://github.com/wlpotter in #1258 comments.
|
I've endeavored to make the recommended changes in the latest commit. Ready for re-review. |
added Syriac placeNames and related bibliographical sources