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

Update Action to Node 16 and handle set-output deprecation #109

Merged
merged 11 commits into from
Dec 23, 2022

Conversation

k3rnelpan1c-dev
Copy link
Contributor

@k3rnelpan1c-dev k3rnelpan1c-dev commented Oct 22, 2022

Description

This PR aims to update this GitHub Action to a recent state.
Its initial aim is to use the "new" Node16 Action runtime as the currently used Node12 runtime is phased out relatively soon (alternative to #107).
It also pins and updates the dependencies used within the Action to their latest versions (ideally this should be handled by something such as Renovate-Bot or Dependabot).
In turn this PR updates to the latest @actions/core, which in turn fixes the deprecation of the set-output command.

Finally, this PR updates all Actions used within the workflows of this repositoy and adds an .editorconfig file to help new contributors.

Related Issue(s)

Checklist

  • This PR includes a documentation change
  • This PR does not need a documentation change

  • This PR includes test changes
  • This PR's changes are already tested

  • This change is not user-facing
  • This change is a patch change
  • This change is a minor change
  • This change is a major (breaking) change*

* it is only major in the sense that it is a major jump from node12 to node16, and GitHub recommending bumping the major version for this

Changes made

  • update Action runtime from node12 to node16
  • update and pin Action dependencies
    • @actions/core from ^1.9.1 to 1.10.0 (addresses the set-output deprecation)
    • @actions/exec from ^1.0.4 to 1.1.1
    • @actions/io from ^1.0.2 to 1.1.2
    • ini from ^2.0.0 to 3.0.1
  • update and pin Action devDependencies
    • @redhat-actions/action-io-generator from ^1.5.0 to 1.5.0
    • @redhat-actions/eslint-config from ^1.3.2 to 1.3.2
    • @redhat-actions/tsconfig from ^1.1.1 to 1.2.0
    • @types/ini from ^1.3.30 to 1.3.31
    • @types/node from ^12 to 16.18.7
    • @typescript-eslint/eslint-plugin from ^4.28.2 to 5.46.0
    • @typescript-eslint/parser from ^4.28.2 to 5.46.0
    • @vercel/ncc from ^0.25.1 to 0.34.0
    • eslint from ^7.30.0 to 8.29.0
    • typescript from ^4.3.5 to 4.9.4
  • fix build error with new TS version
  • add .editorconfig (based on format present in repo)
  • update all actions used in this repositories workflows

@github-actions github-actions bot added the CRDA Scan Pending CRDA scan waiting for approval label Oct 22, 2022
@k3rnelpan1c-dev
Copy link
Contributor Author

sorry for not opening an issue before opening this, if it is necessary I can create one after the fact and I won't have any hard feelings if you choose to close this due to it being a requirement.

Regardless, if this is acceptable and you are interested in receiving similar PRs in the other Action repos I would be more than happy to help out.
Similarly, if you are interested in configuring Renovate-Bot for dependency updates, then I would be happy to help out.

Why Renovate over Dependabot?
Simple, Renovate Bot can group and intelligently update PRs for dependencies where as Dependabot cannot and is rather spamy when it comes to opening and closing bump PRs.

@k3rnelpan1c-dev
Copy link
Contributor Author

Regarding the ESLint, I can fix them, yet I opted to change as little code as possible within this PR as its scope was solely bumping the dependencies up to recent versions.

@k3rnelpan1c-dev
Copy link
Contributor Author

@divyansh42, @jduimovich, @tetchel

sorry for the ping on this, but any chance of getting feedback?
If I can be of any help or need to alter any of this I would be up for helping out 😉

Thank you for your work on all of these actions!

@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented Nov 30, 2022

re-based on the current master and bumped the devDependencies to their current versions

* I shall fix the ESLint errors once this is deemed acceptable as a PR

@der-eismann der-eismann mentioned this pull request Dec 5, 2022
8 tasks
@der-eismann
Copy link
Contributor

Unfortunately it seems this project is kind of dead, but since there was a commit 2 weeks ago there is still some hope. Thanks for the work!

@k3rnelpan1c-dev
Copy link
Contributor Author

it would be very unfortunate if this action would wind up dead as I see podman superior to docker (alone for the oci format support).
I will see to keep this updated and eventually fix the ESLint errors, just so it is an all green CI 😄

@der-eismann
Copy link
Contributor

We see that as well, that's why we forked the repo to get some important changes in. We don't really plan to maintain this though, so it would be nice if this here would become more active. Quite embarrassing for an organization like RedHat if you ask me.

@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented Dec 8, 2022

okay, I tried to fix all the eslint errors (after the eslint updates), however, I have no idea how this ever worked or how it was intended to work:

for (const content of contentToCopy) {
const args: string[] = [ "copy", container, content ];
if (contentPath) {
args.push(contentPath);
}
return this.execute(args);
}

Error:(125, 9) ESLint: Invalid loop. Its body allows only one iteration. (no-unreachable-loop)

The rest was mostly parameter arrangement "problems" (was not sure on the code style target of the project so adhered to eslint's interpretation)

@k3rnelpan1c-dev
Copy link
Contributor Author

okay, I tried to fix all the eslint errors (after the eslint updates), however, I have no idea how this ever worked or how it was intended to work:

for (const content of contentToCopy) {
const args: string[] = [ "copy", container, content ];
if (contentPath) {
args.push(contentPath);
}
return this.execute(args);
}

Error:(125, 9) ESLint: Invalid loop. Its body allows only one iteration. (no-unreachable-loop)

The rest was mostly parameter arrangement "problems" (was not sure on the code style target of the project so adhered to eslint's interpretation)

well, fixed it as well as the logic 😅

Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @k3rnelpan1c-dev for the detailed PR and accept my apologies for delay in the review.
Due to some other priorities, we took too much time to respond.

I have provided a minor review, please take a look.

package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@divyansh42 divyansh42 added the CRDA Scan Approved CRDA scan approved by a collaborator label Dec 23, 2022
@github-actions github-actions bot added CRDA Scan Failed CRDA scan failed unexpectedly and removed CRDA Scan Pending CRDA scan waiting for approval labels Dec 23, 2022
@divyansh42 divyansh42 merged commit 5177407 into redhat-actions:main Dec 23, 2022
@k3rnelpan1c-dev k3rnelpan1c-dev deleted the chore/update-action branch December 23, 2022 22:58
@divyansh42
Copy link
Member

We have created the release for the same.
Should reflect in v2 and v2.11

@k3rnelpan1c-dev
Copy link
Contributor Author

Nice! Thank you for the heads up (Renovate already started notifying me of the new version on some of my repos 🚀)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CRDA Scan Approved CRDA scan approved by a collaborator CRDA Scan Failed CRDA scan failed unexpectedly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]
3 participants