Open weijia-cheng opened 9 months ago
This review is so beautifully formatted! I already expressed my gratitude in a previous post, but I really do appreciate you providing such a thorough and kind review.
You're welcome! I've been doing this for a while now so I've had plenty of practice spotting things and providing feedback :)
Well, the feedback was constructive, and though I have had some experience with Git, it has been some time since I last was actively doing work, which I am going to be using as my excuse for all my poor Git hygiene. I look forward to carrying all the feedback and kindness you have shared with me into future projects!
I believe I addressed most of the changes! The only one I still need to go through is correcting my commit history, which I am realizing now is quite embarrassingly bad. I'll be working on that this evening.
Hey Weijia, I believe I corrected all the issues that were in my original work. When you have the time, I'd appreciate you giving this work another review.
Looks much better! There are a handful of issues remaining:
typogrify
and accept the change. There is a missing word joiner before an em-dash in chapter-31.noblesse oblige
in an <em>
in chapter-7.i1
and i2
in the chapter-28 verse. The indent classes are only meaningful relative to the baseline (no indentation). If the lowest level of indentation is i1
, that means i2
is effectively acting as i1
. What we should do is remove the i1
and change to i2
(don’t forget to delete the now-superfluous i2
rule from the CSS).<b epub:type="z3998:personal-name">
.Looking forward to seeing this get done! Now might be as good of a time as any to mention that I was the one who originally put this on the wanted works list. I was feeling then that having the first producers list be mostly 20s pulp fiction wasn't necessarily going to make the best first impression on everyone (it certainly didn't on me the first time I started contributing to SE!), hence I added Marrow :)
I appreciate you putting this work on the list. Like most areas, there is a deep and necessary need for the presence of diverse voices. If there are any other works, or list of works, that you feel would help make SE a more dynamic place, please let me know!
Also, outside of developing a healthy connection to the SE Manual of Style, are there other works or areas of learning that could help me become a better volunteer/contributor?
The changes look very good, there is only one minor thing left I found, then I can send this over to Alex for final review and publishing.
i2
rule still has to be removed from local.css
.As for advice, I would suggest that, if your goal is to eventually produce The Varieties of Religious Experience (which I am looking forward to by the way! not a book I would read a second time for fun but it definitely had an influence on me the first time around) it would be helpful to familiarize yourself with how some other complicated nonfiction texts have been produced in the past.
From productions I've worked on myself, What Is Art? and What Is Property? have turned out to have very complex formatting and structuring. What Is Art? is especially complex in the way it incorporates extended verse quotations into the text. I haven't looked at the code for this one at all but I would also look at the discussion thread and source code for Principia Ethica; it's a great example of collaboration between the producer, manager, and reviewer on a complex nonfiction book, especially Erin's final notes.
I've read your production of What Is Art! I'll check out the repo and look through the commit history of it. I also saw that SE recently published Principia Ethica and looking at the discussion thread is such a good suggestion. Thank you, Weijia!
Typography
chapter-12
: You have a miscurledyou‘
. (Flagged by lint. Meta note, it’s good practice to run lint before submitting for review to catch things like this.)Spelling
modernize-spelling
removes the space before many instances of’ll
; I’m inclined to accept most of these changes apart fromw’at ’ll
andgin’l ’ll
which already have contractions.chapter-32
:common-sense
was changed tocommonsense
by modernize-spelling, which needs to be reverted because this spelling is for the adjective and not the noun. Modernize-spelling will flag these ambiguous modernizations and these should be manually reviewed after you run that tool.chapter-24
:se find-mismatched-dashes
flags that you have bothter-night
andternight
in your text. It looks liketer-night
is the more prevalent form and theternight
in chapter-24 might have been a mistranscription. You can change it toter-night
(no editorial tag because it does match the scans).z3998:poem
should be replaced withz3998:verse
for your verse quotes, because I believe none of them are quoting the entire poem.z3998:verse
is for fragments, andz3998:poem
is for entire poems.Semantics
chapter-10
: Remove the italics aroundamour propre
; we don’t put italics around words found in Merriam-Webster. Also, it would have bexml:lang="fr"
for French, notla
for Latin ;)sine qua non
,auto-da-fé
,confrères
, etc. Leavemeum
andtuum
though; the only hit in M-W ismeum et tuum
which is not the same thing.chapter-28
: You have an<em>
aroundimpasse
but the italics from the original text were probably because it was being treated as a foreign-language word at the time. In cases like this we remove the italics altogether.mésalliance
has a similar problem.To the Editor of the Everyday Book
in the epigraph seems to be the title of a poem (ref). Hence it should go into quotation marks, and not be in italics. In a editorial commit I would change this toCharles Lamb, “To the Editor of the Everyday Book”
.chapter-16
:was
is still in ani
and not in anem
. I see somei
tags elsewhere that need to be addressed. Use thegrep --recursive --line-number "<i>" src/epub/text/*.xhtml
command to help find these issues. You can check all of your usages of italics withgrep --recursive --line-number --extended-regexp "<i|<em" src/epub/text/*.xhtml
.Code Style
se clean
; it seems there are some whitespace issues. In general it is good practice to run clean before submitting for review.se build-title
is changing some invisible character at the end of your halftitlepage. When I run hexdump before and afterse build-title
I see that the last few bytes,003e
, are changed to0a3e
.U+000A
is the line feed in Unicode, and my interpretation of what is happening is that you were missing the line feed at the end of your halftitlepage and build-title, for some reason, is adding it back in for you. You might want to check what your editor settings are, especially if you are on Windows since I believe Windows has different line termination encoding from Linux.se build-toc
. You get a different table of contents with and without a halftitlepage (I have run into this many times myself when I forget to add a halftitlepage the first time around).se build-images
changes the source image—I am not entirely sure what this is doing but we should accept the change.Colophon
https://jacobzaengle.com
doesn’t link anywhere for me? I can’t connect.Metadata
nacoaf
field should behttp://id.loc.gov/authorities/names/n79138741
(I just stole this from the other Chesnutt ebook, The Conjure Woman.)<meta property="se:url.homepage" refines="#transcriber-1"
)Commit History
These changes will require doing a rebase with git (
git rebase -i
) to update the commit history. People come to SE with a wide range of levels of experience with git. If you need a detailed explanation for any or all of these steps, I will happily provide that.[Editorial]
since it is changing spelling away from the printed text.Meta
More suggestions on how to approach future projects than things to fix.