-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Fix regex for getting project and user #3501
Fix regex for getting project and user #3501
Conversation
Check the .git to the end of the string
Good call. I would definitely add tests somewhere before merging, too. @ericholscher any idea where to add these unit tests? |
For the http clone url
While adding the tests, I note that this really not check for that part of the code, but other part (duplicated code). |
self.pip.repo = 'https://bitbucket.org/user/repo.git' | ||
self.assertEqual(self.version.get_bitbucket_url(docroot='/foo/bar/', filename='file'), 'https://bitbucket.org/user/repo/src/master/foo/bar/file.rst') | ||
self.assertEqual(self.version.get_bitbucket_url(docroot='/foo/bar/', filename='file'), 'https://bitbucket.org/user/repo.git/src/master/foo/bar/file.rst') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to handle this.. Both clone urls are valid for a project with name name.git
, yo can do a clone like git clone https://bitbucket.org/user/name.git
or git clone https://bitbucket.org/user/name.git.git
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But on the web, you only can access to the repo path like https://bitbucket.org/user/name.git/src/master/foo/bar/file.rst
.
https://bitbucket.org/user/name.git.git
gives 404 also a project with name name
(no .git)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, bitbucked gives a url in the form of https://[email protected]/user/name.git.git
for https clone, so I guess this probably never happen (but we need another regex for this case)
I think I can also add tests for ssh clone urls and like this |
@stsewd is this ready for another review? |
@ericholscher yes it is |
Looks good to me. Yay for reducing duplication! |
Fix #1788
This happens when a project name contains
.git
in the middle of the name, likesome.github
,hello.git.hello
.For the record:
name.git
in GitHub, the name changes toname
.name.git
, but the clone url is fine.I want to add some tests to this, but I don't know where to add them.