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.

Sign up for GitHub

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

Jump to bottom

Update CI #491

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Update CI #491

merged 1 commit into from
Apr 25, 2023

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Apr 7, 2023 •
edited
Loading

  • add workflow_dispatch for manual running
  • add fail-fast false in matrix
  • specify persist-credentials: false for actions/checkout
  • update actions to the latest versions
  • move cache before installing the toolchain
  • cache more stuff
  • limit deploy to shssoichiro/oxipng repository
  • reindent

Non-whitespace diff: https://github.com/shssoichiro/oxipng/pull/491/files?w=1

There's one warning which needs to be fixed later regarding set-output:

The set-output command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

The other warning about Node.js 12 actions that were not updated in this PR because there's no newer version, needs to be fixed upstream, e.g. in https://github.com/actions-rs/toolchain:

Node.js 12 actions are deprecated. Please update the following actions to use Node.js 16: actions/checkout@v2, actions-rs/toolchain@v1. For more information see: https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/.

@XhmikosR XhmikosR marked this pull request as ready for review April 7, 2023 05:48
@XhmikosR XhmikosR mentioned this pull request Apr 7, 2023
Merged
XhmikosR
XhmikosR commented Apr 7, 2023
View reviewed changes
.github/workflows/oxipng.yml
key: ${{ matrix.target }}-${{ matrix.toolchain }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ matrix.target }}-${{ matrix.toolchain }}-cargo-registry-
path: |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is taken from https://github.com/actions/cache/blob/main/examples.md#rust---cargo

Let me know if you want me to make any changes :)

@AlexTMjugador
Copy link
Collaborator

AlexTMjugador commented Apr 18, 2023 •
edited
Loading

The other warning about Node.js 12 actions that were not updated in this PR because there's no newer version, needs to be fixed upstream, e.g. in https://github.com/actions-rs/toolchain

The repositories at the actions-rs organization are abandoned, so it's unlikely that they will be updated. I'd suggest migrating to alternative actions, such as dtolnay/rust-toolchain.

@XhmikosR
Copy link
Contributor Author

Yeah, someone else can take care of that later.

@XhmikosR XhmikosR marked this pull request as draft April 23, 2023 05:40
@XhmikosR
Update CI
e2836b9 
* add `workflow_dispatch` for manual running
* add `fail-fast` false in matrix
* specify `persist-credentials: false` for actions/checkout
* update actions to the latest versions
* move cache before installing the toolchain
* cache more stuff
* limit deploy to shssoichiro/oxipng repository
* reindent
@XhmikosR
Copy link
Contributor Author

All right, I made a couple more tweaks. Notes:

  1. clippy seems to fail from forks, but the errors aren't fatal. Ideally it should be skipped when not running from the main repo, see Check failing with "Resource not accessible by integration" actions-rs/clippy-check#2. Another solution would be to move clippy to its own workflow and just run it on pushes only
  2. the https://github.com/actions-rs/toolchain action needs to be replaced by someone more familiar with rust later with https://github.com/dtolnay/rust-toolchain
  3. cache could be used in the deploy workflow too later
  4. bonus: we could upload the binaries for each run so that people can easily try "nightlies"

@XhmikosR XhmikosR marked this pull request as ready for review April 23, 2023 06:02
@shssoichiro
Copy link
Owner

Regarding clippy it seems to still be running the lints against forked repos at least, just failing to post the lints as comments on the PR, which I'm okay with although it would be nice if we could silence the error message.

I'm good with these changes as a starting point. 👍 thanks!

@shssoichiro shssoichiro merged commit e88b9be into shssoichiro:master Apr 25, 2023
@XhmikosR XhmikosR deleted the patch-1 branch April 25, 2023 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@XhmikosR @AlexTMjugador @shssoichiro