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

Fix regex for getting project and user #3501

Merged
merged 8 commits into from Jan 15, 2018

Conversation

@stsewd
Copy link
Member

@stsewd stsewd commented Jan 11, 2018

Fix #1788

This happens when a project name contains .git in the middle of the name, like some.github, hello.git.hello.

For the record:

  • If you create a repo like name.git in GitHub, the name changes to name.
  • GitLab don't allow to create a repo that ends on `.git'
  • Bitbucket allow to create a repo like 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.

Check the .git to the end of the string
@RichardLitt
Copy link
Member

@RichardLitt RichardLitt commented Jan 11, 2018

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
@stsewd
Copy link
Member Author

@stsewd stsewd commented Jan 11, 2018

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')
Copy link
Member Author

@stsewd stsewd Jan 11, 2018

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

Copy link
Member Author

@stsewd stsewd Jan 11, 2018

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)

Copy link
Member Author

@stsewd stsewd Jan 11, 2018

And, bitbucked gives a url in the form of https://user@bitbucket.org/user/name.git.git for https clone, so I guess this probably never happen (but we need another regex for this case)

@stsewd
Copy link
Member Author

@stsewd stsewd commented Jan 11, 2018

I think I can also add tests for ssh clone urls and like this https://user@bitbucket.org/user/repo.git.git (bitbucket only, here the .git is correct)

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Jan 15, 2018

@stsewd is this ready for another review?

@stsewd
Copy link
Member Author

@stsewd stsewd commented Jan 15, 2018

@ericholscher yes it is

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Jan 15, 2018

Looks good to me. Yay for reducing duplication!

@ericholscher ericholscher merged commit bf23b2b into readthedocs:master Jan 15, 2018
1 check passed
@stsewd stsewd deleted the fix-integrations-regex branch Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants