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

A few changes #24

Open
wants to merge 137 commits into
base: master
Choose a base branch
from
Open

A few changes #24

wants to merge 137 commits into from

Conversation

irlittz
Copy link
Contributor

@irlittz irlittz commented Aug 18, 2016

Check the commit messages for more detailed information please. There's a single API-breaking change in the board module: users are now forced to use any of the get_board... functions to instantiate a Board class.

Lawrence and others added 30 commits April 13, 2013 19:50
Antonizoon and others added 16 commits July 21, 2016 19:49
It's like `bumplimit` but for images
…and functions, which is passed down to all requests.

The board API had to be changed a bit: the Board class is now private (_Board), users should use any of the get_board, get_boards or get_all_boards functions to get access to a Board instance. This is to ensure that a timeout for the initial metadata request can be used
…d change their state accordingly. Post now has a new attribute: deleted.
@antonizoon
Copy link
Member

This looks great from my preliminary analysis, I just need dan_'s assistance to test it out, and we'll merge it in and package.

Thanks for the contribution.

@irlittz
Copy link
Contributor Author

irlittz commented Aug 19, 2016

One should also consider if the imports in the __init__.py, besides the get_board... functions are really needed: I can't imagine a scenario where it would be useful to manually instantiate either File, Post, Thread or Url classes as a user. All other modules that are part of the package, import directly from the given module instead of going over __init__.py, so there is no issue there.

@antonizoon
Copy link
Member

I would have to figure out a way to make Board an alias to get_board for compatibility, as a lot of other scripts depend on this library. As for Url, that class does have uses for manual instantiation, though admittedly Post and Thread don't: but there could be situations where it could help.

I've been moving recently, so I will have to look further into this later this week, sorry about that.

@irlittz
Copy link
Contributor Author

irlittz commented Sep 6, 2016

I added a way to keep track of the page the thread was last seen on as best as possible and introduced a few new attributes: page and current_page. Accessing the latter property causes an update to the former by calling the underlying Board instance's get_all_thread_ids-method. Other methods where this information can be easily retrieved have been extended to automatically set the page attribute on the thread object.

Additionally I added some documentation and expanded existing docstrings with information about the new arguments and/or properties introduced in this and earlier commits.

The Board alias for backwards-compatibility has also been added here. Please note that this is as of yet largely untested, i.e. the Board alias and the new page attributes, although I don't foresee any problems. I'll test these a bit more thoroughly if you guys don't get to it first.

@antonizoon
Copy link
Member

Awesome. Yes, that does help us make it possible to merge, though we will still need to test it out when we have time.

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.

6 participants