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.

Sign up for GitHub

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

Jump to bottom

[jungle] removing legacy MusicOwner #715

Merged
merged 1 commit into from
Jul 30, 2014
Merged

[jungle] removing legacy MusicOwner #715

merged 1 commit into from
Jul 30, 2014

Conversation

Turini
Copy link
Member

@Turini Turini commented Jul 29, 2014

using @ManyToMany instead

@Turini Turini self-assigned this Jul 29, 2014
@garcia-jj
Copy link
Member

I love it. Gogogo
On Jul 29, 2014 12:43 PM, "Rodrigo Turini" notifications@github.com wrote:

using @manytomany instead

You can merge this Pull Request by running

git pull https://github.com/caelum/vraptor4 removingMusicOwner

Or view, comment on, or merge it at:

#715
Commit Summary

  • many to many instead of MusicOwner

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#715.

lucascs
lucascs reviewed Jul 30, 2014
View reviewed changes
...-musicjungle/src/main/java/br/com/caelum/vraptor/musicjungle/controller/UsersController.java
*/
@Path("/users/{user.login}/musics/{music.id}")
@Put
public void addToMyList(User user, Music music) {
Copy link
Member

Choose a reason for hiding this comment

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

weird indent

@lucascs
Copy link
Member

lucascs commented Jul 30, 2014

bidirectional ManyToMany relationships are generally not a good idea, specially if the domain tends to evolve and you have to add more attributes to it.

But the code is much simpler. 🐑

@Turini
Copy link
Member Author

Turini commented Jul 30, 2014

this is an example app, the second table to avoid a many to many relationship doesn't help at all :)

Turini added a commit that referenced this pull request Jul 30, 2014
@Turini
Merge pull request #715 from caelum/removingMusicOwner
23b0df3 
[jungle] removing legacy MusicOwner
@Turini Turini merged commit 23b0df3 into master Jul 30, 2014
@garcia-jj garcia-jj deleted the removingMusicOwner branch September 1, 2014 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Turini Turini

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Turini @garcia-jj @lucascs