Support failed to start build locator#19
Conversation
| /** | ||
| * By default failed to start builds are filtered out, call this method to include them. | ||
| */ | ||
| fun withAnyFailedToStart(): BuildLocator |
There was a problem hiding this comment.
'withAnyFailedToStart' name is very confusing. With what? What do you mean be 'any failed'? I would name the method 'includeFailedToStart', but maybe it would be even better to include FAILED_TO_START into BuildStatus. What do you think?
There was a problem hiding this comment.
I'm afraid this will make the option less discoverable and I'm also not sure if there mightn't be a successful build that failed to start. Say, if status had been changed manually.
There was a problem hiding this comment.
Yes, including this value into BuildStatus will make it a little less discoverable. However, adding 3 functions for such a rarely used option will make all other options in BuildLocator less discoverable.
And I've just checked, there is no 'Mark as successful' option in TeamCity UI for failed to start build.
| * By default failed to start builds are filtered out, call this method to include them. | ||
| */ | ||
| fun withAnyFailedToStart(): BuildLocator | ||
| fun withFailedToStart(value: Boolean): BuildLocator |
There was a problem hiding this comment.
Calls like withFailedToStart(false) will look confusing. And why do you need this method at all? If value is false it's just a no-op, and if value is true it's equivalent to the previous method.
There was a problem hiding this comment.
Ok, will rename methods to 'includeFailedToStart()' and 'failedToStartOnly()', thank you! By default failed-to-start builds are ignored.
As a side note, I think there should always be a method to revert an option to default.
There was a problem hiding this comment.
Why do you need to have method to revert options? It would complicate API and may be useful only if someone wants to pass BuildLocator instance between methods. Also if we add it behavior of BuildLocator will become dependent on order in which the methods are called, it may cause problems. Do you have any real use case there it's necessary?
|
This pull request is updated. |
2 participants
No description provided.