Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Trim - WIP #23

Merged
merged 22 commits into from
Apr 11, 2016
Merged

Trim - WIP #23

merged 22 commits into from
Apr 11, 2016

Conversation

BlitzKraft
Copy link
Contributor

Don't merge yet. I need to remove docopts and see if bs4 can be done away with.

@BlitzKraft
Copy link
Contributor Author

There are some major changes. I am considering a version bump to 0.1.0.

What do you think?

Edit: Fully backwards compatible, no new features are added though (except for short hand commands).

@tasdikrahman
Copy link
Owner

Great job @BlitzKraft.

I still see an import of the bs4 module. But aren't you now using the JSON data from their API? So no need to scrape I guess.

Yup. When merged, this would definitely count for a version bump

@BlitzKraft
Copy link
Contributor Author

It's still being used for the dict update.

@tasdikrahman
Copy link
Owner

@BlitzKraft Would it be possible to remove the bs4 dependency?

I think we can request the API for the 0th and the latest xkcd number and then in a loop update the dict. What do you think?

@BlitzKraft
Copy link
Contributor Author

I will work on removing the bs4 dependency later this week. However I'll still be using the archive page instead of going in a loop.

@tasdikrahman
Copy link
Owner

Can update the requirements.txt file?

Merging your PR after that. Looking forward to your next one 😄 @BlitzKraft

@BlitzKraft
Copy link
Contributor Author

Done. And is wheel necessary too?

@tasdikrahman tasdikrahman merged commit b96aeca into tasdikrahman:master Apr 11, 2016
@tasdikrahman
Copy link
Owner

Nope @BlitzKraft Merged it.

@BlitzKraft
Copy link
Contributor Author

I haven't removed wheel, but there is one reference to it in Makefile.

Line 44:     python setup.py bdist_wheel

I haven't removed it this time, but include that in the next PR.

@tasdikrahman
Copy link
Owner

Not an issue right now. I haven't updated this in the pypa. Next PR it is then

@BlitzKraft BlitzKraft deleted the trim branch April 11, 2016 03:14
@tasdikrahman tasdikrahman mentioned this pull request Apr 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants