Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 new UDC to perform an optimized git deep clone #22
Add new UDC to perform an optimized git deep clone #22
Changes from 3 commits
3e1754d
fdb496d
435e52b
3a356b1
90e11fd
ff3c571
dbaae92
da1a867
2e384b4
b86ff15
9a7ebb1
df7781d
8546d9e
68763ce
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vladaionescu should I maybe move
git
andssh
folders into a newtools
folder(and later also DIND_INSTALL into
dind
)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should move all three into such a common / misc / utitlities folder. We can keep DIND_INSTALL in the root too for backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, that's what I was trying to suggest :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved everything under utils, lmk if you prefer a different name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Should
github.com/earthly/lib/utils
expose the UDCs directly? (We can still have the implementation in subdirs)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I wouldn't do it. It just adds more complexity to the language, with little benefit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we're left only with some variation of exposing the UDCs in utils/Earthfile, aren't we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's the best option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the alternative
but this would probably mean cloning more than once, unless it's cached/deduped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a reasonable alternative - especially if sticking with the naming
dind+INSTALL
(as opposed toutils+INSTALL_DIND
). The main downside of this is having to import each UDC separately. Arguably, you might not use multiple lib UDCs for this to be a problem. Maybe it can be in the future?Having utils split up into subdirectories helps with segregating UDCs into different areas of concern nicely. But it also has the drawback that it's slightly more confusing what tag to use for each.
All in all, I'm ok with either option. There are pros and cons for each.