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

Use https links #91

Merged
merged 3 commits into from
Aug 17, 2018
Merged

Use https links #91

merged 3 commits into from
Aug 17, 2018

Conversation

pepicrft
Copy link
Contributor

Resolves #88

Short description 📝

As @dcordero pointed out, GitHub encourages using https links instead of ssh's.

Solution 📦

Replace ssh links with the https version of them.

@pepicrft pepicrft added the type:enhancement New feature or request label Aug 17, 2018
@pepicrft pepicrft self-assigned this Aug 17, 2018
@ghost
Copy link

ghost commented Aug 17, 2018

Danger run resulted in 2 markdowns and 1 message; to find out more, see the checks page.

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Aug 17, 2018

Codecov Report

Merging #91 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #91   +/-   ##
=======================================
  Coverage   84.78%   84.78%           
=======================================
  Files         183      183           
  Lines        6341     6341           
=======================================
  Hits         5376     5376           
  Misses        965      965

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0c92b9...0428ec5. Read the comment docs.

@pepicrft pepicrft merged commit 48cbba3 into master Aug 17, 2018
@pepicrft pepicrft deleted the use-https branch August 17, 2018 10:50
Copy link
Contributor

@dcordero dcordero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, great work, thanks a lot for this change. I just added a minor comment.

@@ -4,7 +4,7 @@ public class Constants {
public static let versionFileName = ".tuist-version"
public static let binFolderName = ".tuist-bin"
public static let binName = "tuist"
public static let gitRepositorySSH = "git@github.com:tuist/tuist.git"
public static let gitRepositorySSH = "https://github.com/tuist/tuist.git"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that it is not longer containing a SSH url, this variable might be refactored to avoid including SSH on its name. I think something as gitRepositoryURL or similar might make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I'll update it. Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants