Skip to content

Failing inheritance fixes#456

Merged
JakeGinnivan merged 8 commits intoGitTools:masterfrom
JakeGinnivan:FailingInheritance
Jun 7, 2015
Merged

Failing inheritance fixes#456
JakeGinnivan merged 8 commits intoGitTools:masterfrom
JakeGinnivan:FailingInheritance

Conversation

@JakeGinnivan
Copy link
Contributor

Builds on @rcknight's pull request at #429. This fixed the perf issues when inheriting branch config and also fixes the failing test in #429

@asbjornu @GeertvanHorrik fyi and code review would be grand

@nulltoken could you have a look at the libgit2 stuff?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using regex for this one? Maybe it's even better to make this a setting (I see some people using "dev" instead on the inet).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A setting for this is already available (as per #292), no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we should use it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed. The new logic should allow not having this fallback. Let me look into it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be removed at the moment. If the code gets to the point where it cannot figure out which branch to inherit from then this code needs to be able to pick between master and develop.

An example:

* [master, develop]
|
* [feature/foo]

In this scenario there is no way to know if GitVersion should inheirit the config from master or develop, this code makes sure that develop gets selected. It isn't great but I can't think of a better solution?

@GeertvanHorrik
Copy link
Contributor

I think it looks good, but I am not 100 % into GitVersion (yet), so might have overseen something.

JakeGinnivan added a commit that referenced this pull request Jun 7, 2015
@JakeGinnivan JakeGinnivan merged commit cf893ff into GitTools:master Jun 7, 2015
@JakeGinnivan JakeGinnivan deleted the FailingInheritance branch June 7, 2015 11:12
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.

4 participants