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

Note for #1145 and style fixes #1150

Merged
merged 11 commits into from
Dec 7, 2019
Merged

Note for #1145 and style fixes #1150

merged 11 commits into from
Dec 7, 2019

Conversation

kungming2
Copy link
Contributor

@kungming2 kungming2 commented Dec 6, 2019

Fixes #1145 and others within models/reddit.

Added a notice per #1145 of the behavior of .moderated() not returning the user's own profile subreddit (u_USERNAME). Style fixes for remaining references to Reddit Gold, which is now called Reddit Premium (note that the attribute for a Redditor is still called is_gold). Also standardizing subreddit prefixes from the legacy /r/ to the official r/.

Standardized the capitalization of Markdown as per the site.

Also updated an oversight in setup.py that says that the minimum is Python 3.4 when it is actually 3.5 per the README. Let me know if I did that wrong.

Also, r/wallpapers still doesn't support the .random() feature. Verified that.

Added note for `moderated()` per the discussion in #1145. Also updated old outdated references to Reddit Gold to Reddit Premium.
Copy link
Member

@bboe bboe left a comment

Choose a reason for hiding this comment

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

Thanks a ton for making these improvements. I added some suggestions for slightly more improvement. Please let me know what you think.

praw/models/reddit/subreddit.py Outdated Show resolved Hide resolved
praw/models/reddit/subreddit.py Outdated Show resolved Hide resolved
praw/models/reddit/subreddit.py Outdated Show resolved Hide resolved
praw/models/reddit/redditor.py Outdated Show resolved Hide resolved
@kungming2
Copy link
Contributor Author

kungming2 commented Dec 7, 2019

@bboe Can I suggest we add another .rst to package_info, something like "style guide"? It would be great if we can standardize people docstrings in general and make sure we're on common ground here going forward.

One example (among many) of something I could see us standardizing is instead of having a host of random subreddits in the Python code examples - test, praw_test, iama, etc standardizing it to one where possible.

@bboe
Copy link
Member

bboe commented Dec 7, 2019

@bboe Can I suggest we add another .rst to package_info, something like "style guide"? It would be great if we can standardize people docstrings in general and make sure we're on common ground here going forward.

I love that idea. Would you like to create such a file?

@kungming2
Copy link
Contributor Author

@bboe I'll have to read up on writing RST but I can definitely try to get it started. Shall I submit it as an Issue then so we can get community input on what the styles should be in the first place?

@bboe bboe merged commit 6eaddb5 into praw-dev:master Dec 7, 2019
@bboe
Copy link
Member

bboe commented Dec 7, 2019

Thanks for the PR. Really loving this work from you.

@bboe I'll have to read up on writing RST but I can definitely try to get it started. Shall I submit it as an Issue then so we can get community input on what the styles should be in the first place?

I think it might be better to start with your opinions. I like this consistency you've provided. If you're up for it, make a PR with your proposed guidelines and we can work from that. I've mostly strayed away from enforcing consistency where it's hard to automate because I don't want to have to nitpick people's PRs for minor changes as it might decrease the chance of getting the PR merged. Of course I agree, having consistency makes the documentation better, so it's a delicate balance.

@kungming2
Copy link
Contributor Author

@bboe Sounds good! I'll write something up with what I think works and submit it as an issue and people can provide input together.

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.

None yet

2 participants