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

Review #1

Closed
6 tasks done
weijia-cheng opened this issue Jul 2, 2023 · 3 comments
Closed
6 tasks done

Review #1

weijia-cheng opened this issue Jul 2, 2023 · 3 comments

Comments

@weijia-cheng
Copy link

weijia-cheng commented Jul 2, 2023

Typography

  • Run se typogrify and accept all changes.

Spelling

  • se modernize-spelling changes “free-handed” to “freehanded” which looks correct to me. Include that change in an [Editorial] commit.

Semantics

  • These words have <em>s that can be removed since they were likely italicized originally as non-English terms, which they are not considered now:
  • rapprochement
  • accoucheurs
  • beau geste
  • debris
  • badinage
  • cul-de-sac
  • patois
  • These words have italics and language inflection that can be removed since they are in Merriam-Webster.
  • ménage
  • per diem (we italicize some Latinisms when they begin with a word that could be confused with an English word, but I wouldn’t consider this one of those cases)

Colophon

  • Instead of
<i epub:type="se:name.visual-art.painting">An Unknown Painting</i>,<br/>
which featured a painting completed in an unknown year by<br/>
an unknown artist.<br/>

use

<i epub:type="se:name.visual-art.painting">Untitled</i>,<br/>
a painting completed in an unknown year by<br/>
an unknown artist.<br/>

Metadata

  • <meta property="se:production-notes">None.</meta> can be deleted since we only include production notes if we have any (c.f. 9.9.4.2).

Suggestions for Future Productions

  • f95f7c0 has a mix of transcription fixes and added semantics. That’s not necessarily wrong, since this commit doesn’t mix editorial with non-editorial changes as far as I can tell, but it’s easier to review when different types of changes are separated out. So ideally for review purposes the transcription fixes would have been the only changes in this commit and the semantics changes would have been in your “Manually add additional semantics” commit. There’s nothing I would change now, but it is worth keeping in mind for the future.
  • I see you added <span xml:lang="it"> around “Signora” which isn’t really necessary since it is in Merriam-Webster, though it seems to make sense in context so it isn’t really wrong either. For comparison we wouldn't ask people to put <span xml:lang="fr"> around “Mademoiselle.”
@jennfrye
Copy link
Owner

jennfrye commented Jul 3, 2023

Thank you so much for reviewing! Everything you mentioned should be corrected now, and I'll make note of these for future reference. I'll keep my commits separated out next time, too.

@weijia-cheng
Copy link
Author

Thanks for making the changes! One last thing though, you still need to change the second line in the colophon snippet I shared with you.

Another note for future productions, we generally don't consider removing italics an editorial change, but I've seen it go both ways so you can leave your commit history how it is for now.

@jennfrye
Copy link
Owner

jennfrye commented Jul 3, 2023

Oops, I should have looked at that more carefully. Fixed now!

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

No branches or pull requests

2 participants