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

Changed provider link as current one returned 404 error #40

Merged
merged 2 commits into from
Nov 9, 2016

Conversation

maciekpaprocki
Copy link
Contributor

No description provided.

@bocharsky-bw
Copy link
Member

bocharsky-bw commented Nov 8, 2016

I don't much like to refer to the list of files, but looks like it's the best option here 👍

@bocharsky-bw
Copy link
Member

bocharsky-bw commented Nov 8, 2016

But what do you think about referring to the https://github.com/thephpleague/oauth2-client/blob/master/docs/providers/league.md instead? It has one more link to the third-party providers inside.

UPD: Ah, docs on GitHub has broken links, that's why I think we need to link their website:
http://oauth2-client.thephpleague.com/providers/league/

@maciekpaprocki
Copy link
Contributor Author

maciekpaprocki commented Nov 8, 2016

Links on this page to third party don't work either, but probably good
shout.

I sent pull request to them too.

Feel like an asshole for doing only commits with link change and getting
everyone notified.

Thanks for quick feedback.
Maciej

On 8 November 2016 at 14:56, Victor Bocharsky notifications@github.com
wrote:

But what do you think about referring to the https://github.com/
thephpleague/oauth2-client/blob/master/docs/providers/league.md instead?
It has one more link to the third-party providers inside


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDQeI7yQ-pHo5iXsYhyddZ05yXuTzl1ks5q8I2WgaJpZM4KsZjQ
.

@bocharsky-bw
Copy link
Member

bocharsky-bw commented Nov 8, 2016

@maciekpaprocki Yep, it's a bit confusing when links change :/

Btw, your PR will be declined. The reason is that broken links on GitHub are working for their website: http://oauth2-client.thephpleague.com/ . So we should use links to this website instead of GitHub. And yes, I know, they have not a perfect solution for building links on this website, but we can't do anything with it. So let's use this one in current PR:
http://oauth2-client.thephpleague.com/providers/league/

@maciekpaprocki
Copy link
Contributor Author

Yes, I agree, I just sent an update.

That's weird that they have such a problem. Surely it's better to look on it on their docs page either way.

Thanks

@bocharsky-bw
Copy link
Member

👍

@bocharsky-bw bocharsky-bw merged commit 18de198 into knpuniversity:master Nov 9, 2016
@bocharsky-bw
Copy link
Member

Thank you @maciekpaprocki for the discussion here and this useful fix!

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.

3 participants