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.

Sign up for GitHub

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

Jump to bottom

refactor(agoric-cli): Improve git clone efficiency #8057

Merged
merged 1 commit into from
Jul 17, 2023
Merged

refactor(agoric-cli): Improve git clone efficiency #8057

merged 1 commit into from
Jul 17, 2023

Conversation

gibson042
Copy link
Member

Description

Use shallow clone to eliminate unnecessary data transfer.

Security Considerations

n/a

Scaling Considerations

Minor improvement for developers on limited networks.

Documentation Considerations

n/a

Testing Considerations

n/a

@gibson042 gibson042 requested a review from dckc July 17, 2023 19:03
dckc
dckc approved these changes Jul 17, 2023
View reviewed changes
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

The code is fine. Shallow clones seem like a good idea here, but...

I have a vague memory that shallow clones have gotchas and should often be avoided, so you might want review from someone with more git expertise.

@gibson042
Copy link
Member Author

In this case the clone is followed a few lines later by orphaning its git data completely with pspawn('rm', ['-rf', '.git'], { cwd: DIR }), so I'm rather confident that this is fine. But I'll add another reviewer for confirmation.

turadg
turadg approved these changes Jul 17, 2023
View reviewed changes
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

sounds right to me

@gibson042 gibson042 added this pull request to the merge queue Jul 17, 2023
Merged via the queue into master with commit bd1190d Jul 17, 2023
59 checks passed
@gibson042 gibson042 deleted the gibson-2023-07-agoric-cli-cleanup branch July 17, 2023 21:27
mhofman pushed a commit that referenced this pull request Aug 7, 2023
@gibson042 @mhofman
Merge pull request #8057 from Agoric/gibson-2023-07-agoric-cli-cleanup
f22c707 
refactor(agoric-cli): Improve git clone efficiency
mhofman pushed a commit that referenced this pull request Aug 7, 2023
@gibson042 @mhofman
Merge pull request #8057 from Agoric/gibson-2023-07-agoric-cli-cleanup
2919839 
refactor(agoric-cli): Improve git clone efficiency
mhofman pushed a commit that referenced this pull request Jan 13, 2024
@gibson042 @mhofman
Merge pull request #8057 from Agoric/gibson-2023-07-agoric-cli-cleanup
76b5690 
refactor(agoric-cli): Improve git clone efficiency
mhofman pushed a commit that referenced this pull request Jan 15, 2024
@gibson042 @mhofman
Merge pull request #8057 from Agoric/gibson-2023-07-agoric-cli-cleanup
3186881 
refactor(agoric-cli): Improve git clone efficiency
mhofman pushed a commit that referenced this pull request Jan 19, 2024
@gibson042 @mhofman
Merge pull request #8057 from Agoric/gibson-2023-07-agoric-cli-cleanup
d6f975e 
refactor(agoric-cli): Improve git clone efficiency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turadg turadg turadg approved these changes

@dckc dckc dckc approved these changes

@michaelfig michaelfig Awaiting requested review from michaelfig

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gibson042 @turadg @dckc