Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

feat(project.clone): add option to preserve namespaces by subdirectories #627

Merged

Conversation

loupgaroublond
Copy link
Contributor

@loupgaroublond loupgaroublond commented Feb 24, 2021

Description

Related Issue

Resolves #[issue_number]

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation
  • Chore (Related to CI or Packaging to platforms)

- adds CLI flag to preserve namespaces on git clone
- sets the target directory to the Project path with namespace when set
- when cloning a group of projects, copy gitArgs to ensure per repo arguments
@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #627 (8e2a4d6) into trunk (054831b) will decrease coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##            trunk     #627      +/-   ##
==========================================
- Coverage   60.51%   60.48%   -0.03%     
==========================================
  Files          90       90              
  Lines        6357     6365       +8     
==========================================
+ Hits         3847     3850       +3     
- Misses       2148     2151       +3     
- Partials      362      364       +2     
Impacted Files Coverage Δ
commands/project/clone/repo_clone.go 61.80% <50.00%> (-1.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 054831b...8e2a4d6. Read the comment docs.

@zemzale
Copy link
Collaborator

zemzale commented Feb 24, 2021

Thanks, for the PR @loupgaroublond
We generally don't accept PRs without an issue attached to them, but this seems, like a nice feature, and does work as expected.

If @profclems is up for this I believe this can be merged into trunk

@loupgaroublond
Copy link
Contributor Author

If you folks can take this as is, that is excellent. Otherwise, happy to open up an issue and go through the change more. There were definitely a few behavioral choices I made unilaterally, that, both, I'd like to change, and I bet others have Strong Opinions on too.

If nothing else, we needed this change at work - so here's the change, it'll get active use off my fork, and whenever you're ready, we can merge it back in.

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

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

@loupgaroublond thanks for working on this! It's a really nice feature.
I'm having a few thoughts on the naming of the --preserve-namespace | p flag.

@zemzale @maxice8 should we maintain that flag name?

@profclems profclems added cmd: repo/project Related to repo or project command enhancement New feature or request labels Feb 24, 2021
@profclems profclems removed the request for review from zemzale February 24, 2021 19:57
Copy link
Collaborator

@zemzale zemzale left a comment

Choose a reason for hiding this comment

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

I don't have a better idea for preserve-namespace, name, but we might go without the p short flag maybe? I don't think that this flag is such a common thing, and there are already a lot of instances of p mostly meaning page for list requests.

@zemzale
Copy link
Collaborator

zemzale commented Feb 25, 2021

This also works with nested groups,
image
Turns into
image

Not counting the nitpick about the short flag LGTM.

@loupgaroublond
Copy link
Contributor Author

I don't have a better idea for preserve-namespace, name, but we might go without the p short flag maybe? I don't think that this flag is such a common thing, and there are already a lot of instances of p mostly meaning page for list requests.

I am ok with dropping the short flag. Please stay tuned for a patch to the PR.

@loupgaroublond
Copy link
Contributor Author

This also works with nested groups,
image
Turns into
image

Not counting the nitpick about the short flag LGTM.

Those nested groups is entirely the motivating use case for me here :)

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

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

On second thought, we can still maintain the -p shorthand for the --preserve-namespace flag since this isn't a list command

Thanks a lot @loupgaroublond for working on this... It will be released as part of v1.16.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cmd: repo/project Related to repo or project command enhancement New feature or request size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants