Skip to content

Conversation

@JakeGinnivan
Copy link
Contributor

Some of these fixes may not be needed because of other fixes, but trying to stamp out any chance of infinite config lookups..

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

I'd rather have too many than too few checks in place to avoid infinite loops, so this looks good to me.

@JakeGinnivan
Copy link
Contributor Author

Ok, looked into this a bit harder. Am now actually excluding branches which have no config or their branch config is inherit. This should mean that we don't resolve a branch with increment of inherit.

I think this is actually the real fix for now.

return matchingBranches.Increment == IncrementStrategy.Inherit ?
InheritBranchConfiguration(context, targetBranch, matchingBranches, excludedInheritBranches) :
matchingBranches;
if (matchingBranches.Increment == IncrementStrategy.Inherit)
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 shouldn't happen now because we filter the conditions which would cause this code to be executed. But still, I think it's worth keeping in just in case

excludedInheritBranches = repository.Branches.Where(b =>
{
var branchConfig = config.GetConfigForBranch(b.FriendlyName);
var branchConfig = config.GetConfigForBranch(b.NameWithoutRemote());
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 I think is the real fix. This will now exclude local and remote branches which do not have config, or the config is Inherit.

Previously we only excluded branches which had config and it was Inherit, but our infinite loops were caused by branches without config..

@JakeGinnivan JakeGinnivan merged commit e4cd2ea into GitTools:master Dec 9, 2017
@JakeGinnivan JakeGinnivan deleted the fix/release-issues branch December 9, 2017 10:10
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