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

Closes #20 - Get a press with a given ID. #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StephaneJuban
Copy link
Contributor

No description provided.

@StephaneJuban
Copy link
Contributor Author

Hi @ches

Just another pull request from the to-do list

@StephaneJuban
Copy link
Contributor Author

Hi @ches
Is this pull request OK for you?
Maybe you could use some help to maintain this repository? :)

@ches
Copy link
Collaborator

ches commented Apr 14, 2016

Apologies for leaving this unanswered for such a long time.

Hmm, this change would break backwards compatibility so it would need to come with a v2.0 version bump—at this point I'd rather not do that just for this minor change.

It's unfortunate because get_press is clearly the right name for consistency with the rest of the library, but the endpoint for getting a single press item by ID didn't exist when it was first written so that's why this naming wasn't anticipated.

Here's what I think the preferable course for me is now:

  • Leave get_press unchanged.
  • Add the new endpoint support with the name get_press_by_id.
  • Alias get_press to also be callable as get_startup_press (get_press_startups is backward naming IMO under the same reasoning as 06c5df3).
  • Have get_press issue a deprecation warning that its behavior will change in the next major version release of the library, and users should migrate to using get_startup_press.
  • For 2.0 we will change get_press_by_id to get_press, and deprecate the former.

Could you make these changes? Please annotate using the @deprecated YARD tag, and I'd be agreeable to adding a helper library with minimal dependencies like deprecations—we do not currently depend even transitively on activesupport so I am not willing to pull it in to use ActiveSupport::Deprecation.

Thanks for your patience!

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.

2 participants