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.

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

Implemented a function for developers to specify the --depth option of the git clone command #60

Closed
wants to merge 1 commit into from

Conversation

yosssi
Copy link

@yosssi yosssi commented Feb 12, 2014

Implemented a function for developers to specify the --depth option of the git clone command. refs #55

@scottferg
Copy link

LGTM

@bradrydzewski
Copy link

I just wrote a bunch of comments and forgot to hit submit. d'oh!

this is good stuff. I'd just like to propose some minor changes.

let's remove the SCM struct and just add the Git struct directly to script.Build. We have not plans to support non-git repositories at this time.

type Build struct {
   ...
   Git *Git `yaml:"git,omitempty"`
}

In the Git struct can we strongly type Depth as an int? The goyaml parser will unmarshal integer fields:

type Git struct {
   Depth int `yaml:"depth"`
}

NOTE: you may need to remove the omit empty, so that it gets populated with a 0, but I'm not sure.

Can we pass the Depth to the Repo as a field value? This is more for consistency, since this how we pass other values used in the Git command (sha, branch, etc):

type Repo struct {
   ...
   Depth int

so the method signature would look like this:

func (r *Repo) Commands() []string {

I appreciate the PR and especially the unit tests!

@yosssi
Copy link
Author

yosssi commented Feb 13, 2014

Thanks for your reviewing.
I close this PR and I will create new one to merge the commits.

@yosssi
Copy link
Author

yosssi commented Feb 13, 2014

See #62.

johannesHarness pushed a commit that referenced this pull request Sep 26, 2023
* FileEditor improvements

* Update clone URL + minor code refactoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants