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] Refactoring #24

Closed
wants to merge 3 commits into from
Closed

[WIP] Refactoring #24

wants to merge 3 commits into from

Conversation

leek
Copy link
Contributor

@leek leek commented Mar 28, 2012

Build Status

Done:

  • Added default controller for /authorize
  • Added missing RefreshToken.mongodb.xml
  • Made configuration naming/style match other FOS bundles
  • Started CouchDB support

Todo:

  • Update unit tests They already pass? Must not be thorough yet...
  • Update documentation
  • Add some better examples
  • Maybe(?) add some edits to Configuration/FOSOAuthServerExtension to prevent BC breaks

Still highly work in progress, but I'd love thoughts/suggestions/criticisms/etc.

Fixes #22, #23, #7, #19

@stof
Copy link
Member

stof commented Mar 28, 2012

@leek There is already a complete refactoring in the bundle in the refactoring branch (not yet completed but @schmittjoh does not have much time for it right now and so is searching someone to help finish it) so we won't do another parallel refactoring

@leek
Copy link
Contributor Author

leek commented Mar 28, 2012

@stof I've seen the branch you're referring to and it is extremely (3 months) behind master in terms of time and features:

  • The unit tests currently fail.
  • It only supports Doctrine ORM.
  • It does not have the recent addition of refresh tokens.
  • It also has unnecessary dependencies on JMSSecurityExtraBundle and JMSDiExtraBundle.

@stof
Copy link
Member

stof commented Mar 28, 2012

@leek the refactoring has been started by @schmittjoh and then stayed pending because of missing time. And it is intended to become the only maintained version of the FOS bundle in the future (the current version is approximately @arnaud-lb's original bundle). So a better way to invest time would be to contact @schmittjoh to help on the refactoring instead of working on another one

@leek
Copy link
Contributor Author

leek commented Mar 28, 2012

@stof Well this version is basically done and has everything on the ToDo list completed.
Personally, I do not want extra unnecessary dependencies.

For anyone who wants a fully functioning OAuth 2.0 bundle (at least until @schmittjoh is done with his branch), I will be maintaining a fork under my namespace here: LeekOAuthServerBundle

@leek leek closed this Mar 28, 2012
@willdurand
Copy link
Member

I think we should consider this PR really important, as there is anything else anyway...

Related discussion: #26

@leek are you still ok to merge your work to the FOSOAuthBundle, as soon as things move?

@willdurand willdurand reopened this Apr 4, 2012
@leek
Copy link
Contributor Author

leek commented Apr 4, 2012

@willdurand Sure. I haven't updated my fork recently but my local version has a few other additions like a base skeleton for requesting a clientId, allowing users to de-authorize a clientId, etc.

/cc #26

@willdurand
Copy link
Member

@leek ok so, I agree you to continue your refactoring here. You should look at the refactoring branch, some stuffs are fine like:

and, maybe:

I agree with you, we don't need any other dependencies.

@willdurand
Copy link
Member

@leek any news?

@willdurand
Copy link
Member

Merged.

Thank you so much @leek!

@willdurand willdurand closed this Apr 14, 2012
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

4 participants