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 RichardLitt added the PR: work in progress Pull request is not ready for full review label Jan 11, 2018
@RichardLitt
Copy link
Member

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 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

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

Copy link
Member Author

@stsewd stsewd Jan 11, 2018

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)

Copy link
Member Author

@stsewd stsewd Jan 11, 2018

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://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 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

@stsewd is this ready for another review?

@stsewd
Copy link
Member Author

stsewd commented Jan 15, 2018

@ericholscher yes it is

@ericholscher
Copy link
Member

Looks good to me. Yay for reducing duplication!

@ericholscher ericholscher merged commit bf23b2b into readthedocs:master Jan 15, 2018
@stsewd stsewd deleted the fix-integrations-regex branch January 15, 2018 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: work in progress Pull request is not ready for full review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants