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

Support upstream League Oauth2 Clients. #3

Closed
wants to merge 8 commits into from

Conversation

nynka
Copy link
Contributor

@nynka nynka commented May 27, 2019

This new feature adds the ability to use any League Oauth2 Client that inherits from League\OAuth2\Client\Provider\AbstractProvider by adding providers of their choice to their local configuration.

  • Are you fixing a bug?

    • Detail how the bug is invoked currently.
    • Detail the original, incorrect behavior.
    • Detail the new, expected behavior.
    • Base your feature on the master branch, and submit against that branch.
    • Add a regression test that demonstrates the bug, and proves the fix.
    • Add a CHANGELOG.md entry for the fix.
  • Are you creating a new feature?

    • Why is the new feature needed? What purpose does it serve?
    • How will users use the new feature?
    • Base your feature on the develop branch, and submit against that branch.
    • Add only one feature per pull request; split multiple features over multiple pull requests
    • Add tests for the new feature.
    • Add documentation for the new feature.
    • Add a CHANGELOG.md entry for the new feature.
  • Is this related to quality assurance?

  • Is this related to documentation?

@nynka nynka marked this pull request as ready for review May 27, 2019 09:20
@nynka
Copy link
Contributor Author

nynka commented May 28, 2019

Ping for review @weierophinney

@nynka
Copy link
Contributor Author

nynka commented Sep 20, 2019

Find any time to review this @weierophinney ? :)

Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

This looks fantastic, @nynka! I've flagged a few documentation changes, but once those are ready, I'm ready to merge this and release a new major version.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
config/oauth2clientauthentication.global.php Show resolved Hide resolved
docs/book/config.md Outdated Show resolved Hide resolved
docs/book/config.md Show resolved Hide resolved
docs/book/config.md Show resolved Hide resolved
docs/book/intro.md Outdated Show resolved Hide resolved
src/OAuth2CallbackMiddlewareFactory.php Show resolved Hide resolved
- Remove implementation and test details in changelog
- Fix typos "Client/client", "s/inherits/inherit/"
Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

The suggestions look good - get them pushed, and I can merge and release!

- Link to the relevant documentation (full URL) in changelog
- Explanation of the "custom" provider
@nynka
Copy link
Contributor Author

nynka commented Oct 3, 2019

The suggestions look good - get them pushed, and I can merge and release!

Consider it done. Thank you for great feedback! @weierophinney

weierophinney added a commit that referenced this pull request Nov 12, 2019
This patch updates the CHANGELOG entries for #3 to reference the pull
request, so users can understand the source of the change.

Additionally, it pushes the new version entry to the top of the file
(CHANGELOG.md files list in reverse chronological order), sets the
version to 2.0.0, and sets the release date.
@nynka nynka deleted the support-upstream-clients branch December 11, 2020 10:57
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

2 participants