Closed quackduck closed 4 years ago
GitHub recommends HTTPS:
Should the guide's steps be changed to HTTPS then? I would be happy to help do that
I usually use HTTPS for Git, so +1 from me. What do the others think?
This page used to recommend HTTPS (linking to the GH guide), then added "but HTTPS might be difficult if you use 2FA, here's how to use SSH" (copying the GH guide info over), then that line was removed, leaving the current contradiction. I don't think 2FA is a problem any more with HTTPS.
So we could recommend HTTPS, and I'd suggest not duplicating too much info from GH guides.
Or maybe show both methods and just mention which one is recommended? Why are we choosing between the methods to show? Shouldn't the user be allowed to choose? Maybe the user wants to use the other method and so has to refer to other guides.
Setting up SSH is a whole topic in itself, and doesn't seem to be present in this guide. Plus HTTPS works with 2FA and other "modern" Git login stuff, and has worked fine for a while now. I'd say just use HTTPS.
Okay, I'll work on shifting the steps over to HTTPS later today. Should I make this a draft or something? I'm new to PRs.
Sure, converting to a draft is a good way to indicate a work in progress :)
Okay, I'm done with adding the HTTPS instructions. Have a look! Is there anything I missed or something that should be added? I did not delete the SSH instructions though.
Hmm... GitHub shows the check as pending even though it has actually completed (I checked by clicking details) I'll make another commit just to see if that fixes it? I'll just add a new line somewhere.
Yep. That fixed it
Any other problems? Any suggestions or edits? What's left to do now?
Getting there... you're up to 11 commits in this PR, though, which wouldn't be very tidy if they were all merged in. How about "squashing" them to a single commit?
Okay, I'll find out how I can do that
Try git rebase -i master
and mark all commits after the first one as "f" for "fixup".
Okay, I squashed the commits into one. (For the first time! I'm surprised it worked 😃 (It did right?)). What's left now?
Maybe replace that "-if you had..." with "- if you had..."?
Aside from that, looks good to me!
LGTM
What does "approved changes" mean? Thanks, @kamleshkc2002 for approving it, I guess...?
What does "approved changes" mean? Thanks, @kamleshkc2002 for approving it, I guess...?
This means that someone has taken a look at your PR and it looks fine to them as code reviewers. The next step would be the collaborator team to review and merge these changes.
Hmm. @apjanke had reviewed as well. @apjanke, is there anything else left to fix before you approve the changes?
Done. Accepted all suggestions and re-squashed
@quackduck Thank you for your contribution!
You’re welcome! Thanks for guiding me through this process, everyone!
By the way, did I get added to the contributor list?
The guide has steps for using SSH and yet a few lines above that, it mentions that it'll use HTTPS. Isn't SSH the new recommended method anyways?