qetza / replacetokens-task

Azure Pipelines task to replace tokens in files with variables.
MIT License
11 stars 2 forks source link

Add aliases for renamed inputs #11

Closed jessehouwing closed 5 months ago

jessehouwing commented 5 months ago

for some reason my pipelines, even though pinned at 5, upgraded to 6 and everything broke. Expectedly, thanks for making this a major upgrade.

But many of the properties were just renamed, have you considered using the aliases option to not break us a much?

I wouldn't mind PR'ing that. It would make the upgrade for most inputs a click settings hit inserts.

https://learn.microsoft.com/en-us/azure/devops/pipelines/tasks/reference/?view=azure-pipelines#what-are-task-input-aliases

qetza commented 5 months ago

Hi @jessehouwing, Strange that your pipelines upgraded to 6 automatically, I didn't see this happened during my tests. I didn't know the aliases property, I'll do a quick PR to add aliases to inputs that were just renamed and release a new path.

jessehouwing commented 5 months ago

The alias works a bit weird, you need to keep the old 'name' property and then put the one you want to use in the YAML as the first item in the aliases array.

jessehouwing commented 5 months ago

Hi @jessehouwing, Strange that your pipelines upgraded to 6 automatically, I didn't see this happened during my tests. I didn't know the aliases property, I'll do a quick PR to add aliases to inputs that were just renamed and release a new path.

It might be because I have a crazy renovatebot config.

qetza commented 5 months ago

Thanks for sharing the aliases feature @jessehouwing, new version 6.0.2 is released 😄

In case you use GitHub workflow i've release an action to do the same replace features https://github.com/marketplace/actions/replacetokens

jessehouwing commented 5 months ago

With the aliases in, you may want to check some of the renamed dropdown items and throw a clear error message, as it may just do weird default behavior otherwise.

Thanks for your quick actions!

ybadragon commented 5 months ago

We have automation in our pipelines to validate these inputs and with v 6.0.2 the actionOnMissing input switched from "fail" to "error" which is now causing issues. v5 used "fail" and v6 uses "error" even though the input is named the same.

ybadragon commented 5 months ago

example code that was working earlier today, but is now broken

if (replaceTokensTasks.some(task => task.getInput("actionOnMissing") !== "fail" && task.getInput("missingVarLog") !== "error")) { this.onValidationFailure("Replace Tokens Task must be set to fail if missing a variable."); }

qetza commented 5 months ago

Hi @ybadragon, As stated in the breaking changes some input names and values in v6 have been renamed.

As proposed by jessehouwing to simplify the work of upgrade i added aliases to inputs that where renamed to minimize the work to be done but the renamed values need to be updated.

In you example actionOnMissing and missingVarLog are both allowed and are the same input but the list of values has changed to error, off or warn as displayed in the log: image

If you cannot change the value it's better then to stay with v5 which has the same features.

ybadragon commented 5 months ago

sorry I may not have explained myself property, this morning the code I provided worked with with both version 5.0 and 6.0, with version 6.0.2 it no longer works. With the addition of aliases when pulling the tasks from a pipeline in azure devops it does not include the missingVarLog property and instead replaces it with the actionOnMissing alias.

qetza commented 5 months ago

Yes you're right that adding the aliases means that you cannot have a single code which will work both with v5 and v6 without adding a check on the version and I didn't had in mind your kind of scenario.

Is it possible to add a check on the version in your code?

ybadragon commented 5 months ago

yeah I can add a check to the code, or change the conditional to support both "fail" and "error" as values. I just wanted to make sure you and anyone else who ran into the issue were aware there was a breaking change between 6.0.0 and 6.0.2

qetza commented 5 months ago

Thanks @ybadragon, yes the version is brand new and i'm trying to fix/improve it quickly as it is not widely use for now and in your specific scenario this patch is a breaking change. I'm not sure this is a common scenario though having a process which validates the pipelines :smile:

jessehouwing commented 5 months ago

This kind of backwards compat is so hard. In the past I've added additional logic to "understand" the old versions of parameters and then logged a warning for people to change their YAML. People never seem to update their pipeline, until you break them forcefully.

I've thought of generating the new YAML snippet and dumping it in the logs. Never got to that unfortunately.

qetza commented 5 months ago

I understand, I've been trying to keep backward compatibility between v3, v4 and v5 which made me add new inputs to enable/switch to new features, make sure to use code that is compatible with node6, node10 and node16... it was becoming a nightmare to maintain.

That's why v6 has a lot a breaking changes, I decided to do a clean start and keep the syntax as close as possible as my new CLI and GitHub Action.