Fix tag-number-pattern and useBranchName configuration options for ContinuousDeployment mode #906
Conversation
|
|
||
| [Test] | ||
| [Category("NoMono")] | ||
| [Description("Won't run on Mono due to source information not being available for ShouldMatchApproved.")] |
There was a problem hiding this comment.
I can't see that ShouldMatchApproved() is used in the test, so the Category and Description attributes can safely be removed, no?
There was a problem hiding this comment.
The first failure I got seemed to be related to the tests running on mono, but you are right it looks like 228082b did pass the build.
|
@simonejsing Since the build ran on Mono just fine, I think you can just |
asbjornu
changed the title
tag-name-pattern configuration option
asbjornu
changed the title
tag-name-pattern configuration option
|
Looks like you were right, the test is enabled again now. Thanks. |
|
Could we instead change the behavior of I don't think we need to support both scenarios. |
|
@JakeGinnivan you mean for the case where the branch is configured in ContinuousDeployment mode? To clarify, that would essentially mean use the same implementation but just use tag-number-pattern instead of introducing a new option. That would work. The reason I opted for the new construct was to avoid breaking existing behavior, but if that is not a concern then I agree with you. |
|
I would say that the current behaviour is a bug because by it's definition continuous deployment should increment each commit. |
|
Should we change the default configuration for pull request branches to be ContinuousDeployment as well then? Today it is set to ContinuousDelivery |
|
I've also noticed that the tag 'useBranchName' does not work when a branch is set to ContinuousDeployment, I'm fixing that as well. |
simonejsing
changed the title
|
New review out, let me know if there are more comments. |
|
Guys can you please review and pull this in? |
|
Heya This may have problems with sorting on NuGet. For example PR number 2 will end up higher than pr 11. How will the packages be consumed? |
|
I see what you mean. Normally it's not a problem since you rarely will consume the latest pre-release packages. I'll add leading zeros like we do for the number. |
|
Please review |
|
@simonejsing Looks good, but your branch is a bit of a mess after all the merges. You should instead use rebase, so you don't get all those merge commits. The pull request will look a lot cleaner then. Think you can clean it up a bit? If you need help on how to get it to a cleaner state, please let me know and I'll be happy to let you know exactly what to write on the command line. 😃 |
simonejsing
force-pushed
the
simon/uniqueprnuget
branch
from
dfad961 to
245e4a1
Compare
|
Okay, looks like I failed pretty bad at rebasing this... |
…in ContinuousDeployment mode. This enables pull request branches to produce unique version numbers when the pull request branch is set to ContinuousDeployment mode.
…nd fix for useBranchName as the tag value
simonejsing
force-pushed
the
simon/uniqueprnuget
branch
from
245e4a1 to
959e84a
Compare
|
That looks better. |
|
@asbjornu I've rebased onto the latest master and force pushed in order to reuse the PR. Let me know if you would like a new PR instead. |
|
@simonejsing Looks much better, thanks! 👍 |
Potential fix for issue #902