git-init: add support for fetching submodules 🚝 #1531
Conversation
It will do those commands by default as they are no-op. Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
googlebot
added
the
cla: yes
tekton-robot
added
the
size/M
| @@ -111,3 +111,45 @@ func Commit(logger *zap.SugaredLogger, revision, path string) (string, error) { | |||
| } | |||
| return strings.TrimSuffix(output, "\n"), nil | |||
| } | |||
|
|
|||
| func SubmoduleFetch(logger *zap.SugaredLogger, path string) error { | |||
| // HACK: This is to get git+ssh to work since ssh doesn't respect the HOME | |||
There was a problem hiding this comment.
This looks like this hack is copied from Fetch(). Opportunity to de-dup by creating a helper func?
There was a problem hiding this comment.
@sbwsg indeed 😛 I was a bit lazy 🤫, I need to enhance that 😉
| @@ -29,6 +29,7 @@ var ( | |||
| revision = flag.String("revision", "", "The Git revision to make the repository HEAD") | |||
| path = flag.String("path", "", "Path of directory under which git repository will be copied") | |||
| terminationMessagePath = flag.String("terminationMessagePath", "/dev/termination-log", "Location of file containing termination message") | |||
| submodules = flag.Bool("submodules", true, "Initialize and fetch git submodules") | |||
There was a problem hiding this comment.
WDYT about surfacing this option all the way up into the Git resource, so users can turn it off if they'd prefer?
There was a problem hiding this comment.
@imjasonh I was waiting for someone to comment this to do it 😛
tekton-robot
added
size/L
|
The following is the coverage report on pkg/.
|
vdemeester
force-pushed
the
1523-submodules
branch
from
5520f42
to
d3c8290
Compare
docs/resources.md
Outdated
| @@ -252,6 +252,9 @@ Params that can be added are the following: | |||
| (branch, tag, commit SHA or ref) to clone. You can use this to control what | |||
| commit [or branch](#using-a-branch) is used. _If no revision is specified, | |||
| the resource will default to `latest` from `master`._ | |||
| 1. `submodules`: defines if the resource should initialize and | |||
| fetch the submodules, value is either `true` or `false`. _If note | |||
| @@ -111,3 +111,45 @@ func Commit(logger *zap.SugaredLogger, revision, path string) (string, error) { | |||
| } | |||
| return strings.TrimSuffix(output, "\n"), nil | |||
| } | |||
|
|
|||
| func SubmoduleFetch(logger *zap.SugaredLogger, path string) error { | |||
| // HACK: This is to get git+ssh to work since ssh doesn't respect the HOME | |||
|
/lgtm |
|
The following is the coverage report on pkg/.
|
Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
vdemeester
force-pushed
the
1523-submodules
branch
from
d3c8290
to
56fb88e
Compare
|
The following is the coverage report on pkg/.
|
|
/lgtm |
|
/test pull-tekton-pipeline-integration-tests |
|
As a new tekton fanboy here at JPL, I'm impressed by the quality of the PR & discussion. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibyom The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
Changes
It will do those commands by default as they are no-op.
Closes #1523
Signed-off-by: Vincent Demeester vdemeest@redhat.com
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes