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

MBS-7307: Show LinkedIn URLs in the sidebar #424

Merged

Conversation

anshuman73
Copy link
Contributor

No description provided.

@chirlu
Copy link
Contributor

chirlu commented Jan 2, 2017

s/linkedIn/LinkedIn/ in the commit description?

@anshuman73
Copy link
Contributor Author

anshuman73 commented Jan 2, 2017

LinkedIn.
That's their official way of calling it. I'll fix this up in a second.

@anshuman73 anshuman73 changed the title MBS-7307: Show linkedIn URLs in the sidebar MBS-7307: Show LinkedIn URLs in the sidebar Jan 2, 2017
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@anshuman73
Copy link
Contributor Author

@y-van-z
There were some problems with regex parsing of linked urls with sub-domains (for example - http://musicbrainz.org/artist/5b7c6d07-4847-43f6-817b-8ea0088c5b92).
Fixed the issue, tested it and squashed the commit for a cleaner history.

@chirlu
Copy link
Contributor

chirlu commented Jan 3, 2017

This is something that should be done in URL cleanup on input. The display code should only deal with cleaned up URLs.

@anshuman73
Copy link
Contributor Author

@uklauer I was talking about the subdomain in the link, ca.linkedin.com
But yes, I agree cleanup also needs to be done (unless you're talking about a way to convert subdomain into something like linkedIn.com/ca/profile-name

@mwiencek
Copy link
Member

mwiencek commented Jan 4, 2017

I think the subdomain should be removed entirely in URL cleanup, since it's not necessary to view the profile (it appears that you can view any profile from any country's subdomain).

As of this writing, there are 305 LinkedIn URLs with two-letter subdomains.

@anshuman73
Copy link
Contributor Author

Created MBS-9188

@mwiencek mwiencek merged commit bfc24e1 into metabrainz:master Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants