-
Notifications
You must be signed in to change notification settings - Fork 86
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 eventName and update event #2483
Conversation
- move "orphan" description from place section to event section - refer to new general section from person-event and place-event parts - include a few sentences that hopefully fit the bill for a generic event description
merge upstream changes
Mostly tweaking content models and examples so that examples are valid. The lone exception is “Platter”, Filip Fabricius’ additional name in the tagdoc for the 'event' element, which is still invalid.
thx @sydb for pointing this out in #2427 (comment)
- make one nested list - ptr instead of ref for internal Guidelines references
clarification in example of Paris terrorist attacks
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.
Technical portions look very good. Overall prose & examples look good, too, but a few suggestions follow.
<p>The <gi>eventName</gi> element is provided in parallel to other naming mechanisms, and especially for <soCalled>canonical</soCalled> | ||
names that are used in texts to mention specific named events. | ||
<egXML xmlns="http://www.tei-c.org/ns/Examples"> | ||
<p>On <date when="1719-03-19">Monday</date>, <rs type="person">she</rs> was writing about the |
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.
Does that general encoding with just the value "person" make sense? At least, the date and the two events in this example are encoded in a more detailed way, i.e. reference to something specific.
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.
Yes, agreed that the use of <rs>
here may be a bit distracting, especially since it's not providing additional info...
Note that this is only instance of the use of <rs>
tagging just a pronoun (i.e. the rest are slightly more specific, including things like "his lady" or "City of Brotherly Love") [NB: this example is duplicated in the elementSpec for eventName, so we'll want to change that there too if we change it here]
</p> | ||
<p>The <gi>eventName</gi> element is used to mark up real and fictitious event names as they appear in the edited text. | ||
As explained with regard to the <gi>event</gi> element (see <ptr target="#NDEVNT"/>), | ||
the use of these elements does not define what might be an important event for a particular edition. Temporal extent, |
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.
same as my comment for line 751: it need not be an edition. Better phrase perhaps "project"?
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 have similar phrasing in #NDEVNT
These guidelines do not prescribe what encoders count as events, nor their granularity, nesting, or amount of other information to record in relation to each event.
And now that I read the rest of this, I think the sentence is a bit broken:
Temporal extent, granularity of events (and hence of their names) are up to the encoders.
IMO, we should delete these sentences and just merge this with the following paragraph:
<p>The <gi>eventName</gi> element is used to mark up real and
fictional event names as they appear in the source. <gi>eventName</gi>
may be used in conjunction with well-defined (e.g. historical) events that are
established in a particular ...
<egXML xmlns="http://www.tei-c.org/ns/Examples" source="#NDGEOGste-Falls"> | ||
<standOff> | ||
<!-- ========= events ========= --> | ||
<listEvent> |
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 a long list of events with, therefore, long lists of related places and persons. Could/Should that be shortened perhaps?
Edit: I see, this is not a new example but it is already in the Guidelines. But still maybe worth shortening for easier reading?
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.
Again, I think we can work on improving the examples in time for the release after this one. When Council discussed this in Paderborn we wanted to prioritize releasing the new content model and encoding, but work on improving the examples later.
(I suggested some changes but then hit the button just for "approve" not for requesting changes.) |
With thanks to @sydb, @sabineseifert, and @ebeshero. Still more to go, but this should get most of the easy stuff
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 will not say there is nothing wrong with this PR, but I will say I did not notice anything that would prevent release.
Many thanks @sabineseifert @joeytakeda @sydb for revising this again. Sorry for being inresponsive, was traveling yet did check occasionally: every change was necessary and appropriate. Many thanks. |
</event> | ||
<event when="1778" ref="http://avalon.law.yale.edu/18th_century/del1778.asp"> | ||
<label>Treaty of Fort Pitt</label> | ||
<desc>Also known as the <name type="event">Treaty with the Delawares</name>, this was the first written treaty |
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.
Okay--I'm reviewing our discussion of this from Paderborn, and I remember Council found this particular example to be problematic.
We wanted to go through and revise the examples (this one in particular) to change <label>
to <eventName>
.
At Paderborn we recommended that <label>
be applied to a change of state, while <eventName>
be used for an event with a name. (Admittedly changes in state like a coronation would probably be given a name, so this may be a little ambiguous.) But in this example, at this particular point, this is the only time we see <name type="event">
while everything else is encoded with <label>
. I think we did intend to change this example to match the new <eventName>
or at least to make the encoding more consistent throughout (as in, apply <label>
to "Treaty with the Delawares" and elevate it to the same position as "Treaty of Fort Pitt". @joeytakeda
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 see that we also recommended simply merging the PR to make <eventName>
available at last, and open a ticket to improve the examples later, so I'm not going to raise this as an objection--just noting it as something we need to revisit! As soon as we merge this PR, we should open a new ticket. (I'll probably just do that.) @joeytakeda
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.
@joeytakeda and all: I'm in favor of merging the PR because the content model and new examples on the <eventName>
specs are important new contributions to the Guidelines. However, I think as soon as we merge this, we need a new ticket (for a later release cycle) to work on updating and improving the Guidelines examples of <event>
in the ND chapter to provide some consistent guidance/examples on when to use <label>
and when to use <eventName>
.
And, as I'm assigned, I'll merge it in now! |
I think, after lots of work and discussion,
eventName
(and the new and improvedevent
) are near ready for showtime 🎉 .This PR should close #2382 and #2466 (with thanks to @djakacki , @skurzinz, @sydb, and @JanelleJenstad !)
You can view the proposed version of the Guidelines here: https://www.sfu.ca/~takeda/tei-guidelines/issue_2382_eventName/html/
Major changes here include:
<eventName>
<event>
s content model and descriptionFor reviewers, if you can, please check the
desc
s inevent
andeventName
as some of will be outdated (or non-existent, in the case ofeventName
) (I'm also not sure if everyone assigned needs to review, but I thought that more eyes wouldn't hurt)