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

Update downloading-multiple-records-using-query-strings lesson to Pyt… #1506

Conversation

frederik-elwert
Copy link
Contributor

Contributes to #1460.

In the course of updating the lesson, I also fixed a few small errors. I removed the newDir function, which was defined but then never used.

@acrymble
Copy link

Thanks for doing so much work @frederik-elwert. I have tried the entire lesson from start to finish and noticed 2 small errors.

First, the following sentence has a typo (which I sure was mine originally):

you are trying to create a director with a * in it (should say "directory")

--

Secondly, the obo.py code chunks have some mistaken duplication in the final few examples. In particular the startValue=0 variable, and the code to create a new directory are accidentally included twice.

@acrymble
Copy link

It looks like this pull request isn't coming direclty from our repository so I can't make the changes myself. If you make the fixes @frederik-elwert I will review and approve.

@acrymble acrymble self-requested a review October 23, 2019 15:52
@frederik-elwert
Copy link
Contributor Author

Thanks for reviewing and catching those! Checked twice, I hope I spotted all the duplicates.

@acrymble
Copy link

Thanks. I'm afraid your fix has introduced a new typo.

There's a double-tab now before "os.makedirs(cleanQuery)" in four code examples. This extra tab should be removed. Otherwise this is ready to republish and thanks for the work @frederik-elwert .

@frederik-elwert
Copy link
Contributor Author

This is weird, I cannot remember touching that part. But thanks for spotting, should be fixed now.

Copy link

@acrymble acrymble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this code and the changes and I believe it is ready to push to the live site. Thank you for all of your hard work and contributions to the project.

@acrymble
Copy link

@svmelton @alsalin As the managing editors, I'll leave this for you to merge. But I approve of the changes.

@svmelton svmelton merged commit 5fdc92b into programminghistorian:gh-pages Oct 25, 2019
@frederik-elwert frederik-elwert deleted the downloading-multiple-records-py3 branch October 31, 2019 09:51
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

Successfully merging this pull request may close these issues.

4 participants