Skip to content

⚡ proposed fixes - #57

Open
rneswold wants to merge 1 commit into
mainfrom
optimize-rust
Open

⚡ proposed fixes#57
rneswold wants to merge 1 commit into
mainfrom
optimize-rust

Conversation

@rneswold

Copy link
Copy Markdown
Contributor

This was generated by Gemini. It proposes to fix two issues with the previous version:

  • It built the code coverage executable each time it was run. Since the utility is written in Rust, it added quite a bit of time to the workflow.
  • It unpacked and rebuilt the cache each time, yet I've never saw it use the contents. I've only ever seen it unconditionally build the Rust project (twice! once for clippy and for the unit tests)

Gemini proposes that this version uses the cache properly, so it should only rebuild crates that are updated. It also uses a workflow that has a prebuilt instance of the code coverage tool so we don't need to build it over and over.

This PR is being made to share the changes with others that are more familiar with GitHub workflows. It looks good to me, but that's not saying much.

This was generated by Gemini. It proposes to fix two issues with the
previous version:

- It built the code coverage executable each time it was run. Since
  the utility is written in Rust, it added quite a bit of time to
  the workflow.
- It unpacked and rebuilt the cache each time, yet I've never saw
  it use the contents. I've only ever seen it  unconditionally build
  the Rust project (twice! once for clippy and for the unit tests)

Gemini proposes that this version uses the cache properly, so it
should only rebuild crates that are updated. It also uses a workflow
that has a prebuilt instance of the code coverage tool so we don't
need to build it over and over.

This commit is being made to share the changes with others that
are more familiar with GitHub workflows. It looks good to me, but
that's not saying much.
uses: actions/cache@v5
continue-on-error: false
- name: Setup Rust Cache
uses: Swatinem/rust-cache@v2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To reduce security risks, I recommend to use onlhy github actions from github or verified creators.
See an example of a recent vulnerability : https://semgrep.dev/blog/2025/popular-github-action-tj-actionschanged-files-is-compromised/

@rneswold

Copy link
Copy Markdown
Contributor Author

I don't know if we should merge this or simple use it for discussion on improving the workflow. @jacob-curley-fnal and I have a discussion in his PR which might reject this PR altogether.

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.

2 participants