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

Fix bug on 'Creating and Viewing HTML files with Python' #1997

Merged
merged 5 commits into from
Feb 1, 2021

Conversation

rivaquiroga
Copy link
Member

@rivaquiroga rivaquiroga commented Jan 27, 2021

Closes #1777 + propagates changes to Spanish translation

Checklist

  • Assign yourself in the "Assignees" menu
  • Assign at least one individual or team to "Reviewers"
    - [ ] if the text needs to be translated, assign the relevant language team(s) as "Reviewers" and tag both the team as well as the managing edtor in your PR. Please follow the translation request guidelines when writing your PR description
  • Add the appropriate "Label"
  • Ensure the Travis CI checks pass
  • Check the live preview of your PR on Netlify
  • If this PR closes an open issue, add the phrase Closes #ISSUENUMBER to the description above

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.

@rivaquiroga rivaquiroga marked this pull request as ready for review January 27, 2021 20:42
@rivaquiroga rivaquiroga requested review from a team and removed request for svmelton and JoshuaGOB January 27, 2021 21:13
@walshbr
Copy link
Contributor

walshbr commented Jan 27, 2021

A few things:

Those lines were specifically introduced as an attempt to fix an earlier error. Here is the one that introduced the 'b' - #1630. It was a proposed solution here - #407. So can you just confirm that the things work as expected and run on windows with the change? I don't understand windows file formats enough to be able to comment.

But the other thing to note is that, as these files are part of the Python sequence, you also need to update with the changes in these files - /assets/python-lessons.zip and python-es-lecciones.zip. There is a different zip file for each python lesson, and IIRC, you'll need to just make sure you update the zip files following the one in which the error appears.

And the 'wb' usage actually appears in several places.…I'm trying to get a list to you. It would be great to be able to grab all of those now.

@walshbr
Copy link
Contributor

walshbr commented Jan 27, 2021

Screen Shot 2021-01-27 at 4 27 18 PM
This is a list of the files and the instances in the number of times that 'wb' appears in them. Let me know if that doesn't make sense?

@walshbr
Copy link
Contributor

walshbr commented Jan 27, 2021

We might have just been pointed to the two files you pinpointed based on where the bugs were reported, but they'll be issues throughout the Python sequence.

@rivaquiroga
Copy link
Member Author

Yes, it was checked on Windows. I'll check the zip files. Thanks for noting that!
Regarding the rest of the lessons, all of them are going to be checked: #1885 (comment)

@walshbr
Copy link
Contributor

walshbr commented Jan 27, 2021

Awesome - thank you!

@rivaquiroga
Copy link
Member Author

Regarding the previous change that added the 'b', I suspect that we trusted the solution proposed by a reader without checking it on Windows 😬

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 accessing files as binary
3 participants