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

UserService works with trait GenericProfile instead of BasicProfile #477

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

Conversation

leonidv
Copy link

@leonidv leonidv commented Sep 28, 2014

Reasons:

  1. Scala doesn't allow to extends case class from case class.
  2. Coding into interfaces is good OOP practices

Reasons:
1. Scala doesn't allow to extends case class from case class.
2. Coding into interfaces is good OOP practices
@jeantil
Copy link
Contributor

jeantil commented Sep 29, 2014

+1

@leonidv
Copy link
Author

leonidv commented Sep 29, 2014

Hmm, I think this is corrupted pull request. Not all changes are pulled.

@jaliss
Copy link
Owner

jaliss commented Sep 29, 2014

@leonidv I was about to add a comment on this. I do not see any changes related to the UserService here.

@jeantil
Copy link
Contributor

jeantil commented Sep 29, 2014

I +1oned the idea I haven't checked the implementation

@leonidv
Copy link
Author

leonidv commented Sep 29, 2014

Yeah, I'm so sorry. I did refactoring but lost my changes. I'll try to make good commit today.

Reasons:
1. Scala doesn't allow to extends case class from case class.
2. Coding into interfaces is good OOP practices
Fixed some compiles problems (checked via sbt publishLocal)
@leonidv
Copy link
Author

leonidv commented Sep 29, 2014

Fixed commits.

@nightrise
Copy link

Nice job! Any chance you could merge this, jaliss? Would love to have this in the official release!

@leonidv
Copy link
Author

leonidv commented Oct 11, 2014

Yesterday I understood that it's not complete refactoring. Providers create BasicProfile from oauth information. Now I think how can to fix it.

@leonidv
Copy link
Author

leonidv commented Dec 2, 2014

This pull request is ready for review. I was wrong when thought that it has some problems.

By the way, it also has fix as in #481 pull request.

@albertchau
Copy link

+1 Would be very helpful! I have a complex User class

@gicmo gicmo mentioned this pull request Jan 20, 2015
- changed project name to SecSoc
- added author (Leonid Vygovskiy)
- removed java-demo module
AuthenticationException is required message string now.
@leonidv
Copy link
Author

leonidv commented Jan 29, 2015

My last commits (from 87863bf) it's not for pull request. I just not power github user and don't know how to separate pull request changes from my fork changes.

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

5 participants