BASC-py4chan icon indicating copy to clipboard operation
BASC-py4chan copied to clipboard

A few changes

Open irlittz opened this issue 8 years ago • 5 comments

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.

irlittz avatar Aug 18 '16 22:08 irlittz

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.

antonizoon avatar Aug 19 '16 04:08 antonizoon

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.

irlittz avatar Aug 19 '16 14:08 irlittz

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.

antonizoon avatar Aug 22 '16 13:08 antonizoon

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.

irlittz avatar Sep 06 '16 01:09 irlittz

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

antonizoon avatar Sep 06 '16 04:09 antonizoon