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 submodule init issue #2588

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Fix submodule init issue #2588

merged 1 commit into from
Jun 5, 2024

Conversation

soheilshahrouz
Copy link
Contributor

Solves this issue

Since sockpp uses ssh to clone the repo, ssh keys need to be set up. Alternatively, we can use https for cloning the repo.

@soheilshahrouz
Copy link
Contributor Author

Related to this PR is this stackoverflow answer

@AlexandreSinger
Copy link
Contributor

Ah I see what happened here. When you use a git@ url for the repo, it requires that you have a git shh key setup (which is a good idea btw). If you use the https:// version you do not; however if you wish to push to the repo you will not be able to login. @soheilshahrouz I think you have a good point turning this into a https:// since the general user will just be pulling (not pushing anything); and so we would not require people to have an ssh key if they want to build VTR.

I am excited to see what gnarly things this will do once people rebase this change. But it must be done.

@vaughnbetz
Copy link
Contributor

Seems logical since we do this for other submodules. @w0lek let us know if you have any concerns.

@w0lek
Copy link
Contributor

w0lek commented Jun 5, 2024

Seems logical since we do this for other submodules. @w0lek let us know if you have any concerns.

@vaughnbetz
i am fully ok with that change.
FYI: i will update end user documentation PR comments very soon, i am keeping an eye on it, just busy with other stuff right now.

@vaughnbetz vaughnbetz merged commit 434f529 into master Jun 5, 2024
102 checks passed
@vaughnbetz vaughnbetz deleted the fix_submodule_init_issue branch June 5, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants