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

Add discontinuation notice to Multireddit.rename() #1091

Merged
merged 2 commits into from
Jul 10, 2019

Conversation

Pyprohly
Copy link
Contributor

Multireddit.rename() always results in a 400 response now since Reddit’s rebranding of multireddits as “Custom Feeds”. We should at least document the discontinuation if .rename() isn’t to be removed just yet.

Renaming a multireddit can still be achieved through Multireddit.update(), although this is change is only shown on the redesign UI and is not reflected in the URL.

References

@jarhill0
Copy link
Contributor

Considering the endpoint no longer works behind the scenes, I think in this case we should just remove the method (and associated tests, etc). This is what we did with Subreddit.submissions() in #916. While it does break semver, there's no use in leaving a non-working method in.

@bboe
Copy link
Member

bboe commented Jun 28, 2019

I agree. Let's just remove it. When the upstream API breaks, it's a bug for PRAW to have broken code, so I would argue it's not a violation of SEMVER to remove a method that's no longer working.

@Pyprohly
Copy link
Contributor Author

Pyprohly commented Jul 1, 2019

Okay, I’ve removed the method and moved the explanation to the git message.

LMK if I need to do anything more.

Someone who isn’t me needs to check what travis is complaining about. It should have passed.

@bboe
Copy link
Member

bboe commented Jul 4, 2019

Please rebase off of master and then the issues should be resolved.

@bboe
Copy link
Member

bboe commented Jul 10, 2019

@Pyprohly ping

@Pyprohly
Copy link
Contributor Author

I now know of two other discontinued endpoints, and their broken PRAW methods. I think I’m going to add them to this PR and remove them all in one swoop soon.

Unless there’s a reason to accelerate merging this low priority patch, please be patient.

This method no longer works due to a change in the Reddit API.
The name in the URL can no longer be altered, but the name as
shown on the UI can still be updated via `Multireddit.update()`.
Use `.. versionadded::` sphinx markup
Remove `__all__` in models/__init__.py
@Pyprohly
Copy link
Contributor Author

Scratch that. One was a false alarm. The other I’ve just included in another PR.

Merging this now.

@Pyprohly Pyprohly merged commit efcdddd into praw-dev:master Jul 10, 2019
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.

3 participants