colagrosso / theodore-roosevelt_the-rough-riders

Other
0 stars 0 forks source link

Review issues #1

Open vr8hub opened 2 years ago

vr8hub commented 2 years ago
vr8hub commented 2 years ago
vr8hub commented 2 years ago
colagrosso commented 2 years ago

I haven't started transcribing Appendices E, F, and G yet, but I thought I would update my progress on the other tasks here. I completed them all and took the liberty of checking them off above. My commit history matches the task order pretty closely, but I can link to specific commits if there's a question of whether I addressed something you raised.

Also, thank you very much for correcting my approach in Appendix B:

For Appendix B, I don't think the material surrounding the letters is a letter.

I was way off there, and I didn't notice the subtle changes in which parts were set off from the rest of the text. Also, thank you for finding the roughriders0000unse IA scans. I found those easier to read, and they of course have the other appendices. I added them as a second <dc:source>.

vr8hub commented 2 years ago

Thanks for the update, great progress! Yes, the checkboxes were for your convenience, so that's perfect. I'll review it again once you're done, so don't worry about commit links.

Unless there's something in the original scans that isn't in the new ones, you don't need both sources; just the second one is sufficient if that's the one you're going with.

colagrosso commented 2 years ago

Ok, I added Appendices E, F, and G from the scans (as D, E, and F here), so this is ready for you to review again.

There wasn't anything in the original scans that isn't in the new ones, I also took your advice and listed just roughriders0000unse as the only source.

colagrosso commented 2 years ago

One additional reviewer note: There's some extra styling in the scans—mostly italics—that I didn't translate to appendix-d.xhtml and appendix-e.xhtml. Examples would be the titles of the senders of letters, such as:

Major-General United States Army.

or a different example:

To be Brigadier-General by Brevet.

I'm open to feedback on adding more styling and/or what semantics to add. You might have given this feedback above already for Appendices A and B, and if so I'm sorry I missed it. Once I get them right, I'll make them all consistent.

vr8hub commented 2 years ago

Great work, Mike! I'm about to have to leave for an engagement this evening, but I wanted to give you what I've found so far.

Sorry to have to leave without finishing, but I'll try to get back to it tomorrow or Monday!

colagrosso commented 2 years ago

Thanks a bunch for the quick feedback, and I totally understand you're not done. I addressed what you noted so far.

A few comments:

vr8hub commented 2 years ago

Sorry for the confusion—yes, I just meant the appendices didn't need to be children of a separate "Appendices" section.

Good job catching the eyewitness and roman numeral in the new appendices; I saw them at a glance yesterday, but forgot them by the time I did the write-up. :)

I think the only piddly things left are:

After that, I think you're good to go. Once again, great job; there was a lot of tricky stuff here, especially on all the appendices.

colagrosso commented 2 years ago

Right on, thanks. Those last four changes were small, but they made the formatting look a lot better. It's good to know those approaches.

Thanks a bunch for the review, and double thanks for your contributions to the tools. They're great.