add: add support for git sources #5438
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: danishprakash The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
danishprakash
force-pushed
the
add-git-ref
branch
4 times, most recently
from
3a3abfc
to
3d0f34a
Compare
Signed-off-by: danishprakash <danish.prakash@suse.com>
danishprakash
force-pushed
the
add-git-ref
branch
2 times, most recently
from
aba9551
to
3a83d68
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
danishprakash
force-pushed
the
add-git-ref
branch
2 times, most recently
from
1236851
to
da7f372
Compare
Signed-off-by: danishprakash <danish.prakash@suse.com>
danishprakash
force-pushed
the
add-git-ref
branch
from
da7f372
to
92ed66a
Compare
|
Quick fly by review looks OK to me, but you have test issues and a rebase is needed. |
You meant packit issues? Because tests seems to be fine afaict |
| // github.com/containers/buildah.git | ||
| // github.com/containers/buildah.git#main | ||
| // github.com/containers/buildah.git#v1.35.0 | ||
| var gitURLFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`) |
There was a problem hiding this comment.
Not sure this is necessary, and MustCompile on init variables slows things down on startup.
There was a problem hiding this comment.
can we use Delayed() from the containers/storage/pkg/regexp package?
| } | ||
| getErr = copier.Get(cloneDir, cloneDir, getOptions, []string{"."}, writer) | ||
| closeErr = writer.Close() | ||
| if renameTarget != "" && renamedItems > 1 { |
There was a problem hiding this comment.
Line 456:
renamedItems := 0
Line 476:
if renameTarget != "" && renamedItems > 1 {
Is renamedItems == 0 when evaluating the if statment in line 476? (It doesn't look like anything is modifying renamedItems)
What type of PR is this?
What this PR does / why we need it:
adds support for git sources with the ADD directive
How to verify it
Which issue(s) this PR fixes:
Fixes #4974
Special notes for your reviewer:
Does this PR introduce a user-facing change?