Automate publishing crates to crates.io #1364
Conversation
There was a problem hiding this comment.
This looks mostly good. Could you use cargo publish -p PACKAGE-NAME instead of setting the working directory? I know I suggested changing the working directories in 5085c5b, but I think using cargo publish -p PACKAGE-NAME is slightly cleaner. As an added bonus one could just copy+paste the steps and run them from the project's root to publish manually if we avoid modifying the working directory.
Honestly when it comes to situations like this I think it's fine to basically test in production. In other words, just trigger the workflow when we're ready and expect it to work. If there's anything to fix we can make a new patch release to re-trigger the workflow. I've even seen big projects like Vue make new patches just to re-trigger a previously broken CD. Though of course we would never merge in a broken CD workflow 😉 |
.github/workflows/cd.yml
Outdated
| CARGO_TERM_COLOR: always | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v3 |
There was a problem hiding this comment.
| - uses: actions/checkout@v3 | |
| - uses: actions/checkout@v4 |
.github/workflows/cd.yml
Outdated
| env: | ||
| CARGO_TERM_COLOR: always |
There was a problem hiding this comment.
I think that this environment variable can actually be set on the workflow level (root level), not the job level. That way it will be set for all jobs, which should help on build steps for other jobs.
There was a problem hiding this comment.
I’ll give it a try! This is how it was done in the “old” implementation, but that doesn’t make it optimal. Documentation supports your claim: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#env
.github/workflows/cd.yml
Outdated
| - name: Install Rust | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
| profile: minimal | ||
| components: clippy | ||
|
|
There was a problem hiding this comment.
All of actions-rs is deprecated right now. TBH I'm wondering if we actually need a step to install Rust here. GitHub Actions have Rust and cargo installed by default, and typically a pretty recent version AFAIK. I never bothered to explicitly install the Rust toolchain in some personal projects, and I've never had a problem.
There was a problem hiding this comment.
Interesting. Looking more closely, the existing deploy job has the following “Install Rust” step:
onefetch/.github/workflows/cd.yml
Lines 17 to 21 in ac7e2b8
I suppose it makes sense to either match that in cargo-publish, or remove both “Install Rust” steps. What do you think?
There was a problem hiding this comment.
Let's add dtolnay's action for consistency.
This mostly reverts commit 5085c5b. The goal is to restore `cargo-publish` automation, and fix handling of workspace crates in a follow-up commit.
Fixes o2sh#1363 by ensuring crates are published on a platform that fully supports symbolic links. Based on a suggestion in o2sh@5085c5b#commitcomment-91354413.
musicinmybrain
force-pushed
the
automate-publish
branch
from
8e51c0f
to
477921c
Compare
I’ll follow up with this change next. |
|
I think I handled all the feedback except for #1364 (comment). Let me know what you think about #1364 (comment), and I’ll be back to this in a few hours. |
Great! Once you resolve #1364 (comment) this will be good to me. |
Done! Thank you for the thorough and helpful review. |
spenserblack
changed the title
This mostly reverts 5085c5b, and attempts to support publishing workspace crates
onefetch-ascii,onefetch-image, andonefetch-manifest. This would fix #1363 by ensuring crates are published on a platform that fully supports symbolic links.I have little to no experience with GitHub Workflows, and I am unable to test this PR, but it still seems like it should do the job.