Skip to content

Conversation

@walshbr
Copy link
Contributor

@walshbr walshbr commented May 21, 2020

closes #1768

Checklist

If you are having difficulty fixing Travis errors, first consult https://github.com/programminghistorian/jekyll/wiki/Making-Technical-Contributions carefully, especially "Common Travis Errors". Then contact the technical team if you need further help.

@walshbr walshbr requested review from a team May 21, 2020 19:52
@walshbr walshbr self-assigned this May 21, 2020
@walshbr
Copy link
Contributor Author

walshbr commented May 21, 2020

This is something of a thorny pull request. It corrects an error in the Python introductory series that was introduced in updating from Python 2 to Python 3. It's thorny because 1) the lesson sequence and the error was introduced early in the sequence and 2) the lessons each contain zip files that contain all the code for the lessons up until that point. So the one error meant that it popped up in a couple dozen places.

The lessons have all been translated, but the error itself shouldn't affect any translated text - just the code blocks. I'm tagging the French and Spanish teams because it looks like most if not all of these have been translated. I've tagged both of those teams in case there were lessons in the pipeline that needed to be modified. @jenniferisasi and @spapastamkou - it might be easiest for me just to go through and edit the code blocks for those lessons and re-upload the zip files. Does that work for you? I can let you know if I run into anything that requires actual translation. You can do it yourselves if you want, but feels like it might just be quicker for me to do it since I've already been swimming in the code for these lessons this afternoon.

@walshbr walshbr marked this pull request as draft May 21, 2020 20:04
@walshbr
Copy link
Contributor Author

walshbr commented Jun 1, 2020

@jenniferisasi and @spapastamkou - just pinging you again on this. Is it alright for me to go through and edit the code blocks for the Spanish and French lessons pertaining to the Python series?

@rivaquiroga
Copy link
Member

Yes! If you run into anything that requires actual translation, let us know.

@spapastamkou
Copy link
Contributor

Yes, @walshbr, thank you. I also ping @fdlaramee as we work on the Python series so he is aware as well.

@fdlaramee
Copy link
Contributor

Thanks @walshbr and @spapastamkou. FYI, as I was translating Downloading Web Pages with Python, I noticed that the inline code had been updated but that some of the explanatory text still referred to Python 2 documentation and modules; I will open a separate ticket to fix the English text once you are done with this one.

@walshbr
Copy link
Contributor Author

walshbr commented Jun 1, 2020

KK I'll get to work on updating the other zips and files and ping if I have any questions.

@walshbr
Copy link
Contributor Author

walshbr commented Jun 1, 2020

Sounds good @fdlaramee. It looks like none of the lessons affected by this lesson have been translated into French, now that I look more closely. My French isn't very great, but it does look like some of these lessons might be in the process of being translated on ph-submissions. So just be advised. Thanks!

@walshbr walshbr marked this pull request as ready for review June 1, 2020 19:46
@walshbr
Copy link
Contributor Author

walshbr commented Jun 1, 2020

Should be all set for merging.

@programminghistorian/spanish-team - there wasn't any translation required (I just copied the Spanish from the code blocks) but please take a look at the changes here - https://github.com/programminghistorian/jekyll/pull/1785/files - to make sure I didn't mess anything up?

@programminghistorian/french-team - can you just approve this to acknowledge that you've taken into account any changes that might need to be made to lessons in progress?

@walshbr
Copy link
Contributor Author

walshbr commented Jun 15, 2020

@programminghistorian/spanish-team reminder to approve this please.

@svmelton and @rivaquiroga I think we need to determine as per https://github.com/programminghistorian/jekyll/wiki/The-Programming-Historian-Digital-Object-Identifier-Policy-%28April-2020%29 if these constitute major changes (based on the definitions there it looks like they do) and, if so, request new DOIs?

@acrymble
Copy link

@walshbr I think we're ok here to merge, yes?

@walshbr
Copy link
Contributor Author

walshbr commented Jun 22, 2020

@acrymble it depends on whether we can agree these changes are minor and don't need new DOIs. If that's the case then this can be merged. If they are major we'd need to get new DOIs and do the new lesson file iteration things. I'd also note that in the future this is probably something we should decide before actually making the revisions because the policy expects you to keep the old lessons intact and make new lessons. My bad, because I wasn't familiar with the new policy yet.

@acrymble
Copy link

I believe MEs have to make that call?

@walshbr
Copy link
Contributor Author

walshbr commented Jun 22, 2020

@acrymble Yeah I pinged @svmelton and @rivaquiroga above. I think it's probably a question of whether any code changes constitute a major change. Or whether minor bug fixes like these are sort of on par with broken link / spelling errors.

@drjwbaker
Copy link
Member

@acrymble Yeah I pinged @svmelton and @rivaquiroga above. I think it's probably a question of whether any code changes constitute a major change. Or whether minor bug fixes like these are sort of on par with broken link / spelling errors.

Policy is at https://github.com/programminghistorian/jekyll/wiki/The-Programming-Historian-Digital-Object-Identifier-Policy-%28April-2020%29 with some examples of minor/major changes. The idea was that as new minor/major examples pop up, MEs populate the wiki so that other MEs have a reference point.

@drjwbaker
Copy link
Member

Note also the step for the Major changes:

If the article has not been copyedited (this will apply to most lessons published before Spring 2020), pass it through the Copyediting Process.

The idea here was that as pre-Spring 2020 articles went through Major changes, we may as well copy edit them. This has the potential to throw EN/ES/FR/PO articles lightly out of sync, but is at the discretion of MEs to decide.

@rivaquiroga
Copy link
Member

This one seems to be a minor change, like broken links / spelling error. It is a bug from the major code update made before the DOIs were assigned, but not a major code change itself.

@walshbr
Copy link
Contributor Author

walshbr commented Jun 22, 2020

@drjwbaker - oooooh I didn't realize we'd be adding to that list. I must not have read closely enough. I was wondering about it. Thanks.

re: @rivaquiroga - "This one seems to be a minor change, like broken links / spelling error. It is a bug from the major code update made before the DOIs were assigned, but not a major code change itself."

It'd be worth adding to that to the wiki then if @svmelton agrees, because this example has been a question I've had.

Would definitely reiterate that in the future we should decide on major/minor before updating the lessons. Because it'd be a bit of a bear to go back and pull out the old versions of the lessons to retroactively copy them as per the major change guidelines.

@drjwbaker
Copy link
Member

@drjwbaker - oooooh I didn't realize we'd be adding to that list. I must not have read closely enough. I was wondering about it. Thanks.

That is okay. It is new. And if MEs don't like it we can change it.

It'd be worth adding to that to the wiki then if @svmelton agrees, because this example has been a question I've had.

Yes please. With a link back to the ticket/PR if possible.

Would definitely reiterate that in the future we should decide on major/minor before updating the lessons. Because it'd be a bit of a bear to go back and pull out the old versions of the lessons to retroactively copy them as per the major change guidelines.

Yes. Ideally, ME decides minor/major first. This new policy/workflow is one of the consequences of the DOI work, and our DOI provider would very much appreciate that we don't on one hand undermine the spirit of the DOIs they are generating for us (so, make major changes without retiring the old version and creating a new version with a new DOI), and on the other hand don't send them loads of DOI requests for small tweaks. It is a tricky balance, but with experience and examples I'm sure we'll get used to it.

@drjwbaker
Copy link
Member

Final point: we drafted this policy (with Matt) but have yet to test it because there was no example to test it with, so capturing and sharing knowledge - and then editing the policy - is vital.

@acrymble
Copy link

I think then @walshbr all are agreed that we can merge?

@walshbr
Copy link
Contributor Author

walshbr commented Jun 25, 2020

@acrymble - I assumed we needed all the impacted managing editors to sign off, in which case we are waiting on @svmelton to confirm that these don't constitute major changes.

@walshbr
Copy link
Contributor Author

walshbr commented Jun 25, 2020

But if just @rivaquiroga's word is good enough to speak for ME's in general then yeah we can merge.

@acrymble
Copy link

@walshbr ok can we try to be really explicit with pull requests and tickets when we need someone to check something? I think we're all often too vague and the person we're waiting for doesn't even realise it. This is increasingly important as we diversify linguistically. Nuanced language may be missed.

@walshbr walshbr requested a review from svmelton June 25, 2020 14:02
@walshbr
Copy link
Contributor Author

walshbr commented Jun 25, 2020

OK - I added her to the list of reviewers. Sorry - I pinged her a few times here asking in the conversation above. I just assume she just hasn't had a chance to look at it yet.

@acrymble
Copy link

@walshbr yes, I see you did. I'm just suggesting being really clear if a ticket is waiting specifically for someone before you can do your own action. You said you assumed we needed @svmelton to respond. That's fine. But you can equally read the discussion in a way that suggests we don't anymore.

If we all are really explicit, then we can action tickets quicker and hopefully help people see quickly what they need to do in less time. Similar to the https://github.com/programminghistorian/jekyll/wiki/Requesting-Translation-Guidelines. I'm raising this with you here, but it's something we all do. So I hope you don't feel singled out.

@svmelton
Copy link
Contributor

Yes, we're good to go!

@walshbr walshbr merged commit 15f818d into gh-pages Jun 25, 2020
@walshbr walshbr deleted the python-lessons-update branch June 25, 2020 15:04
@drjwbaker
Copy link
Member

@walshbr If you think this is worth adding to the list of 'minor' edits on https://github.com/programminghistorian/jekyll/wiki/The-Programming-Historian-Digital-Object-Identifier-Policy-%28April-2020%29, please do.

@walshbr
Copy link
Contributor Author

walshbr commented Jun 26, 2020

Done @drjwbaker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python lesson sequence and urllib.request output bug

8 participants