Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove version input #103

Merged
merged 15 commits into from
Aug 26, 2021
Merged

Remove version input #103

merged 15 commits into from
Aug 26, 2021

Conversation

korthout
Copy link
Owner

@korthout korthout commented Jul 27, 2021

Version input removal

This removes the need for the version input, and deprecates it in the action.yml. From now on, users no longer need to provide this input. These changes are backwards compatible with workflows defined for previous versions.

Reasoning

It was pretty strange that users had to state twice which version of the backport-action they wanted to use, AND that both of these had to be in sync. Specifically, the version of the tag in the uses property and the version input had to be the same value.

How

The version input is dropped with this changeset. This is possible because it was only used to find the backport.sh script that was used to perform the actual backporting. This script was hard to maintain and its separation from the typescript code made it hard to refactor it. By replacing the script with individual git commands executed from typescript using execa, we could remove the sh file, which removed the need for the version input.

korthout added 10 commits July 20, 2021 15:27
This makes it easier to reuse the script in tests that don't use the
same exec library
We need to remove all references to the backport.sh script before we can
remove it
Now that backport.sh is no longer used directly, we can remove the
function
The version was used to find the backport.sh script. Since the script is
no longer used as a file, but inlined, the version is no longer
necessary.
The version input is no longer used, so users no longer have to provide
it as input. In order not to break things for existing users, we simply
deprecate the input by making it optional.
Version input is deprecated and no longer used for anything.
@korthout korthout force-pushed the remove-version-input branch from 810b288 to 7e873d5 Compare July 27, 2021 15:42
@korthout korthout force-pushed the remove-version-input branch from 7e873d5 to 0cb7433 Compare July 27, 2021 15:55
@korthout
Copy link
Owner Author

Current approach is not supported by @actions/exec see actions/toolkit#461.
Will have to further split up the script in separate commands that can be executed individually

In order to split the script into individually executable commands, a
new git.ts file is introduced. Since these changes also required changes
to the acceptance tests (the script could no longer be executed as a
whole), this commit also replaces @actions/exec and child_process with
execa. This library makes executation of commands a bit easier and can
be used inside a github action as well as locally in tests.

While making changes, I was tempted into replacing the use of git
worktree for cherry-picking. Error handling has also changed a bit,
because in the script this was handled by `-x` flag, but must be done
explicitly in typescript.

Lastly, the acceptance test has had some minor changes, to improve
analysing failing tests.
@korthout korthout force-pushed the remove-version-input branch 2 times, most recently from fe215aa to 0822ea4 Compare August 26, 2021 12:33
Instead of first switching to the target branch and then creating a new
branch to checkout, we can simplify these actions using the git command:
`git switch -c <branch> <start-point>`.
@korthout korthout force-pushed the remove-version-input branch from 0822ea4 to 4e4e600 Compare August 26, 2021 12:51
@korthout
Copy link
Owner Author

Successfully tested with https://github.com/zeebe-io/automation-lab/pull/73

@korthout korthout merged commit cb15591 into master Aug 26, 2021
@korthout korthout deleted the remove-version-input branch August 26, 2021 13:01
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.

1 participant