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

[WIP] Add deprecate notes #40

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

Conversation

stephane-monnot
Copy link
Contributor

@stephane-monnot stephane-monnot commented Aug 18, 2016

Improve display of deprecated errors:

  • Unique error messages
  • report after tests report
  • report with Red BG

Method to deprecate:

  • friends()
  • getMutualFriends()
  • getFriends()
  • getFriendsOfFriends()
  • ...

Fix all deprecated errors for the build:

  • Remove use of deprecated methods in src
  • Use the group annotation in test classes

Add more details about new methods in deprecated notes:

  • @deprecated comments
  • E_USER_DEPRECATED error messages
  • in a changelog file

New methods (lazy getters = return a collection of model, a model instance or a integer for count method):

FriendRequests

  • Add deprecated note migration getFriendship become getFriendRequest (only 1 way ?)
  • Add deprecated note migration getAllFriendships become getFriendRequests (only 1 way ?)
  • Add deprecated note migration getPendingFriendships become getPendingFriendRequests (only 1 way ?)
  • Add deprecated note migration getAcceptedFriendships become getAcceptedFriendRequests (only 1 way ?)
  • Add deprecated note migration getDeniedFriendships become getDeniedFriendRequests (only 1 way ?)
  • Add deprecated note migration getBlockedFriendships become getBlockedFriendRequests (only 1 way ?)
  • Remove deprecated note getFriends

Model (ex: User)

  • Remove deprecated note getMutualFriends
  • Remove deprecated note getMutualFriendsCount
  • Remove deprecated note getFriendsOfFriends
  • Remove deprecated note getFriendsCount

@stephane-monnot stephane-monnot force-pushed the feature/deprecate branch 2 times, most recently from 0f3e9cd to f82317f Compare August 18, 2016 19:47
@hootlex
Copy link
Owner

hootlex commented Aug 18, 2016

@stephane-monnot I believe we should keep getSOMEfriendships and getSOMEfriends methods just to exist and reference them in a Lazy Getters section in the readme. What do you think?

@stephane-monnot
Copy link
Contributor Author

Yes, but maybe rename getSOMEfriendships to getSOMEfriendRequests.

@stephane-monnot
Copy link
Contributor Author

@hootlex Could you valid the roadmap ("new method" checklist above) for deprecated notes and answer the "only 1 way" questions please ?

@hootlex
Copy link
Owner

hootlex commented Aug 19, 2016

By keeping the current getSOMEfriendships method names, we achieve a small backward compatibility. So, I think it's better to keep them.

About '1 way', I agree. They should work how they used to work to avoid confusion.

@stephane-monnot
Copy link
Contributor Author

Yes, but I'm a little confused about friendships and friends.

@hootlex
Copy link
Owner

hootlex commented Aug 19, 2016

Keep the getSOMEFriendRequests if you prefer them. The difference is small.

@hootlex
Copy link
Owner

hootlex commented Aug 22, 2016

@stephane-monnot let me know when this is ready to merge.

@stephane-monnot
Copy link
Contributor Author

I have to implement new methods because we can't mark methods as deprecated without alternatives.
I will try to finish this week.

@stephane-monnot
Copy link
Contributor Author

@hootlex Could you work on this PR and :

  • Implement new renamed methods
  • Add details on deprecated messages (You should use XXXX instead)

I can work a little on this PR this month, but I don't have time as before, because I'm teaching in a school.

@hootlex
Copy link
Owner

hootlex commented Oct 3, 2016

@stephane-monnot I am very happy to hear that. 😄 I don't have a lot of time now too because I am working on my Vue.js book.

Though, I expect to start working on this upgrade next month. Also, I will create a Gitter channel to be able to discuss about the package.
Thanks for the work you've done so far.

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