normalize all github urls

[patrickkettner]

fixes #7

an alternative to the regex solution proposed in #79

[desandro]

I'd like to give this PR some attention. Thanks for fulfilling our request!

https://github.com/bower/registry/pull/79#issuecomment-50245688

After messing with this for a little bit, the code is way uglier. the url parsing doesn't give much, since it doesn't normalize anything. I still have to go through the entire regex, just one piece of the protocol at a time basically.

This code looks much cleaner. Is it missing the regex you mentioned?

[patrickkettner]

I'm glad you like this. to me it looks more complicated, but I can see how that regex could be a beast for people to get over.

which ever ya'll like is fine by me

[rayshan]

Thanks Patrick, I like this a little better too. Just a tiny bit of regex.

I just pulled some git hosting stats out of the registry:

Total # of bower registry packages as of 7-30-14 - 17055
github public - 16947 (99.4%)
github enterprise (e.g. github.paypal.com) - 50
bitbucket - 14
gist.github.com - 12
gitorious - 2
beanstalk - 1
code.google.com - 1
gitlab - 1

Could you add a test or 2 for urls that don't contain github?

[patrickkettner]

sure!

did we want to normalize gist urls? What sort of tests do you imagine?

[rayshan]

If we do that, then should we normalize all valid urls? It may be too much effort for too little gain. I'm ok either way.

For test, I was just thinking about adding "should pass through non-github urls".

[patrickkettner]

If we do that, then should we normalize all valid urls?

Eh...Git accepts a lot of URLs. like..a lot. I don't think that validating all urls would be a worthwhile enterprise. But normalize the couple of common ones (github, gist, bitbucket, etc) makes sense to me, personally.

[rayshan]

Totally agree. I'm ok with just github url support given the stats. There's bigger :fish: to fry :wink:

[patrickkettner]

your the boss, boss :]

are we good with this one, or are there more tweaks needed?

[rayshan]

Did you have a chance to add a test for "should pass through non-github urls"? Other than that LGTM, ready for another owner to merge, I don't have write access.

[patrickkettner]

sorry for taking so long, totally forgot to push up my changes. all squishey and pushied now, though

[rayshan]

Thanks Patrick. LGTM ready for owner to merge.

[sheerun]

lgtm2

[patrickkettner]

thanks fellers!

[sheerun]

Do we also want to normalize existing urls?

[patrickkettner]

I figured that could happen when the migration to the new backend could happen, but it wouldn't take more than a few seconds to update the current db

[desandro]

Merrrrrrged. I've deployed this commit c4a6842 to Heroku. This is a bit outside my area of expertise. But in the spirit of getting things done: it is done. LMK if I borked something

@patrickkettner Thanks so much for the work here!

@rayshan I've added you to the Registry team. Welcome! I appreciate your recent involvement with the issues here. Next time, you can merge it :grin:

[rayshan]

Thanks @desandro! I'll be careful.

Maybe the Registry team should have this repo as part of the team? Still don't have write access.

[sheerun]

@desandro Registry team is for editing the registry entries, not registry code.

I added @rayshan to the bower team which has write access to bower repos.

[rayshan]

Thanks guys

[desandro]

ooooooh :relieved: