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

Replace checkout with switch/restore #791

Merged
merged 3 commits into from Mar 16, 2021

Conversation

@outsideris
Copy link
Contributor

@outsideris outsideris commented Oct 24, 2020

Overview

TL;DR
Replace checkout with switch and restore because the latest git encourages to use switch/restore instead checkout.

Questions

Next Steps

I can't change PDF files, because I don't know how I can generate them.

Review

I tried to change them in all supported languages using Google Translate.
But I'm not sure because I can read only Korean and English.

Signed-off-by: Outsider <outsideris@gmail.com>
@brianamarie
Copy link
Contributor

@brianamarie brianamarie commented Oct 28, 2020

Thank you for this PR, @outsideris!

Before merging, I'd like to confirm with the @github/ps-implementation-engineers that this change works for them. We may also want to make sure this change is implemented in the training manual for consistency.

@github/ps-implementation-engineers, what do you think?

cc @github/ps-programs

Copy link
Member

@parkerbxyz parkerbxyz left a comment

Thank you for these contributions, @outsideris!

I can’t vouch for the non-english translations, but the commands looks great!

@brianamarie Can you verify that is it safe to make changes to Git commands in the non-english versions before we merge? I know some repos take care of the translations in a separate process, using the english version to reference what needs to be updated. Not sure if that’s the case here.

@parkerbxyz
Copy link
Member

@parkerbxyz parkerbxyz commented Dec 9, 2020

We may also want to make sure this change is implemented in the training manual for consistency.

This is on our radar. 😎

@rwnfoo
Copy link
Contributor

@rwnfoo rwnfoo commented Dec 23, 2020

Thank you @outsideris for helping with this! 🥳

There are 2 commands that need to be straightened out, which looks like you're aware of:
git switch for branches that already exist and git switch -c to replace git checkout -b. However, there is an error in the original docs where the command for creating a new branch is git checkout instead of git checkout -b, this is one example. So just going by the old docs to change to switch isn't going to cut it.

I can only read 2 other languages where this does not line up, so I made suggestions in the Malay and Chinese versions. But I think it needs to be corrected for other languages too.

@outsideris
Copy link
Contributor Author

@outsideris outsideris commented Dec 27, 2020

@rwnfoo Good point!! I will change other languages as well even I can read only English and Korean.

outsideris and others added 2 commits Dec 27, 2020
Co-authored-by: Rowena Foo <rwnfoo@github.com>
Signed-off-by: Outsider <outsideris@gmail.com>
@outsideris
Copy link
Contributor Author

@outsideris outsideris commented Dec 27, 2020

I fixed creating a branch with -c option. I checked it by translating them on Google Translate.

@rwnfoo rwnfoo requested a review from parkerbxyz Jan 4, 2021
@rwnfoo
Copy link
Contributor

@rwnfoo rwnfoo commented Jan 4, 2021

Thank you so much @outsideris for the work!

@parkerbxyz - Wondering if we should hold on this PR until git switch is no longer an experimental feature.

Copy link
Member

@parkerbxyz parkerbxyz left a comment

Looks good. Thanks for your contributions, everyone!

@parkerbxyz
Copy link
Member

@parkerbxyz parkerbxyz commented Mar 16, 2021

@parkerbxyz - Wondering if we should hold on this PR until git switch is no longer an experimental feature.

@rwnfoo Let’s proceed here like we did in githubtraining/training-manual#281 and ship it! :shipit:

@parkerbxyz parkerbxyz merged commit 453df53 into github:master Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants