Aws contributes special variable definitions - #61
Conversation
|
|
||
| namespace Sashimi.Server.Contracts.Variables | ||
| { | ||
| public class ActionSpecialVariable |
There was a problem hiding this comment.
I'm not sure about the name. These could be Action or Account or a number of other things in the future, right? Could this just be SpecialVariable?
There was a problem hiding this comment.
Happy to rename this. We also have SpecialVariable on the server-side that also have a CustomBindingLabel. We could call both SpecialVariable or make the server-side one SpecialVariableDefinition?
| public ActionSpecialVariable(string name, BindingBehaviour userVisible = BindingBehaviour.Visible, params string[] deprecatedAliases) | ||
| { | ||
| Name = name; | ||
| DeprecatedAliases = deprecatedAliases; |
There was a problem hiding this comment.
Why do we need this DeprecatedAliases property?
There was a problem hiding this comment.
There are a number of variables in server that have been renamed over time. The deprecated aliases contain the list of previous names, and during the deployment pipeline server will duplicate the current variable value into all of the old names as well, in case any existing scripts are using them. I.e. is for backward compat.
There was a problem hiding this comment.
hmm ... are we planning to/can we address this backward compact issue? If this can never be addressed somehow, maybe it should be considered as a feature here to support Aliases rather than a DeprecatedAliases as we usually use attribute-based approach for deprecation/obsolete instead of polluting the object itself to avoid any unnecessary API change of the object in the future.
| } | ||
| } | ||
|
|
||
| public enum BindingBehaviour |
There was a problem hiding this comment.
Should this just be a boolean instead? e.g. IsVisibleToUsers
There was a problem hiding this comment.
This is a bit like TinyTypes, having the enum makes the call very clear as to what is happening and provides compile safety. new SpecialVariable("foo", BindingBehaviour.Hidden) is easier to reason about reading the calling code than new SpecialVariable("foo", false)
There was a problem hiding this comment.
hmm ... sounds like we are trying to get rid of any usage of boolean for better readability, am I right?
Is Visibility a better name than BindingBehaviour ?
ab62a1f to
e535a49
Compare
3 participants
Server PR: https://github.com/OctopusDeploy/OctopusDeploy/pull/6356
This includes moving
IContributeSpecialVariablesandActionSpecialVariablefrom Server.