Skip to content

Expose NoFetch to fix #646 #653

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

Merged
merged 1 commit into from
Sep 24, 2015
Merged

Conversation

clairernovotny
Copy link

No description provided.

@clairernovotny
Copy link
Author

Confirmed this works on VSO if I then set GitVersion_NoFetchEnabled to true in the build parameters

JakeGinnivan added a commit that referenced this pull request Sep 24, 2015
@JakeGinnivan JakeGinnivan merged commit 039a43b into GitTools:master Sep 24, 2015
@clairernovotny clairernovotny deleted the nofetch branch September 24, 2015 02:19
@clairernovotny
Copy link
Author

Do you think the targets should detect if they're running on an agent that requires nofetch (like VSO) and set the default value to true? Then it'd "just work" for more people but it does put extra logic in the targets.

@JakeGinnivan
Copy link
Contributor

No the targets, the targets is like command line switches.

I forget where I wrote this, but my plan was to add bool? CanFetch to https://github.com/GitTools/GitVersion/blob/master/src/GitVersionCore/BuildServers/IBuildServer.cs#L12 or something similar. Because each build server implementation knows if it needs dynamic repositories or cannot fetch

@clairernovotny
Copy link
Author

It is a better idea to have a the IBuildServer tell you if it needs fetching. If it's set there and used anywhere the NoFetch parameter currently is as a default if NoFetch isn't explicitly set, then that's a better solution.

I was just thinking that people shouldn't have to specifically override things on the happy path :)

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.

2 participants