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

check release repository urls are https and have .git extensions #201

Closed
wjwwood opened this issue Sep 5, 2013 · 9 comments · Fixed by #233
Closed

check release repository urls are https and have .git extensions #201

wjwwood opened this issue Sep 5, 2013 · 9 comments · Fixed by #233
Milestone

Comments

@wjwwood
Copy link
Contributor

wjwwood commented Sep 5, 2013

Currently when users are prompted for the release repository url on the first release for a distro, they can put what ever they want. If it is github, then we should ensure it is https and .git extended. Other tools in the toolchain should be more robust to this, but I'll enforce it in bloom for consistency.

@dirk-thomas
Copy link
Member

Please consider addressing this soon to avoid pull request like this: ros/rosdistro#2990

@mikepurvis
Copy link
Contributor

What's the specific need for .git extension?

@wjwwood
Copy link
Contributor Author

wjwwood commented Feb 4, 2014

I forget, @dirk-thomas do you remember? I remember running into this problem before and for some reason the build farm needed the extension or maybe some conditional logic requires it to differentiate something.

@tfoote
Copy link
Member

tfoote commented Feb 4, 2014

I believe that github is smart enough to to give you what you want, but
many of the standard hosting tools will not differentiate the incoming
client agent and serve the webpage instead of the git repo if you use the
url without .git extension.

@dirk-thomas
Copy link
Member

I can't recall the case we had before but we need the .git extension somewhere in the pipeline of the farm I think.

@mikepurvis
Copy link
Contributor

Okay. I'm all for consistency—I just wasn't sure if there was something actually breaking due to it. I'm pretty sure the Clearpath repos are all over the map with respect to including the suffix or not.

Rather than scold users until they get it right, I suggest a regex match -> transform -> confirmation prompt. Eg:

  • User pastes in URL which contains "github.com"
  • Regex match to extract user/org and repo name
  • Create url of form "https://github.com/org/repo.git"
  • If URL does not match what was entered, prompt to confirm that this is an okay substitute.

More of an implementation pain, but better end experience, especially for new users of Bloom.

@wjwwood
Copy link
Contributor Author

wjwwood commented Feb 18, 2014

I opened #233 to address this. It does not do the more advanced parsing and recommendations that @mikepurvis suggested, but it will check for .git as a suffix and https:// as a prefix, warn if it doesn't have them, and allow them the opportunity to correct the url before continuing.

@wjwwood
Copy link
Contributor Author

wjwwood commented Feb 21, 2014

Also adding a rosdistro CI check:

ros/rosdistro#3204

@wjwwood
Copy link
Contributor Author

wjwwood commented Feb 21, 2014

This is an example of where the .git extension is expected in our toolchain: ros-infrastructure/rosinstall_generator#18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants