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

Add ability to pull student repos issue #8 #39

Merged
merged 6 commits into from
Oct 3, 2023
Merged

Conversation

shanep
Copy link
Contributor

@shanep shanep commented Sep 19, 2023

What are you trying to accomplish?

Change the default behavior when cloning student repos to skip any repos that have already been cloned. Add in a command line flag to allow existing repos to be sync'd instead of a full clone.

Fix issue #14
Implements Feature #8

What approach did you choose and why?

Instead of doing a fresh clone every single time, skip any repos that have already been cloned and give the user a new command line flag to sync. If the user wants a fresh clone then they can already use the --directory flag to do so.

UPDATE: This change should take some pressure off your endpoint :)

What should reviewers focus on?

I couldn't see anywhere in the documentation where the gh cli allows you to pass a path when performing a sync so I had to change the working directory. If there is a better way to do a sync with the gh client please let me know :)

Change the default behavior when cloning student repos to skip any
repos that have already been cloned. Add in a command line flag to
allow existing repos to be sync'd instead of a full clone.

Fix issue github#14
Implements Feature github#8
Copy link

@nuke-web3 nuke-web3 left a comment

Choose a reason for hiding this comment

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

Will help a lot for using this tool for sure! 😍

Copy link
Collaborator

@zrdaley zrdaley left a comment

Choose a reason for hiding this comment

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

@shanep thank you for this contribution!

I was chatting with @RyanHecht and in order to be consistent with typical git commands, it would make more sense to have a gh classroom pull command rather than a --sync flag on the gh classroom clone command.

If you could take some time to make this into it's own command then I'd be happy to re-review.

@shanep
Copy link
Contributor Author

shanep commented Sep 26, 2023

@zrdaley no problem! I like that approach its keeps the semantics all lined up and doesn't overload the clone command :) I will get it all updated as soon as I can :)

Copy link
Contributor Author

@shanep shanep left a comment

Choose a reason for hiding this comment

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

@zrdaley Changes have been pushed to the branch and are ready for review :)

@shanep shanep requested a review from zrdaley September 26, 2023 22:56
@shanep shanep changed the title Add new command line flag to sync repos Add ability to pull student repos issue #8 Sep 26, 2023
Copy link
Collaborator

@zrdaley zrdaley left a comment

Choose a reason for hiding this comment

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

I tested the command locally and it worked as expected 🎉

However, to ensure that this is aligned with the clone command, should we have the ability to pull starter-repos as well? i.e.

gh classroom pull starter-repos
# OR
gh classroom pull student-repos

@shanep
Copy link
Contributor Author

shanep commented Sep 29, 2023

Updated to align with the clone command for both student repos and starter code :)

@shanep shanep requested a review from zrdaley September 29, 2023 22:06
Copy link
Contributor Author

@shanep shanep left a comment

Choose a reason for hiding this comment

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

I think this should be good to go now. However, please let me know if you still see any issues :)

zrdaley
zrdaley previously approved these changes Oct 3, 2023
Copy link
Collaborator

@zrdaley zrdaley left a comment

Choose a reason for hiding this comment

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

Small nit, but otherwise LGTM! Thank you for this contribution.

Co-authored-by: Zenara Daley <zrdaley@github.com>
Co-authored-by: Zenara Daley <zrdaley@github.com>
@shanep
Copy link
Contributor Author

shanep commented Oct 3, 2023

Small nit, but otherwise LGTM! Thank you for this contribution.

No worries :) Truly appreciate the feedback! Excited to see this get merged.

@zrdaley zrdaley merged commit 2d9c71d into github:main Oct 3, 2023
4 checks passed
@zrdaley
Copy link
Collaborator

zrdaley commented Oct 3, 2023

Once https://github.com/github/gh-classroom/actions/runs/6395315615 is green you should be able to upgrade the CLI and use this command.

@shanep shanep deleted the add-sync branch November 7, 2023 14:50
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.

3 participants