Skip to content

ci: unify GHA build process#1172

Merged
kanadgupta merged 8 commits intonextfrom
kanad-2025-02-19/esbuild-instead-of-terser
Feb 27, 2025
Merged

ci: unify GHA build process#1172
kanadgupta merged 8 commits intonextfrom
kanad-2025-02-19/esbuild-instead-of-terser

Conversation

@kanadgupta
Copy link
Copy Markdown
Contributor

@kanadgupta kanadgupta commented Feb 19, 2025

🧰 Changes

a few small behind-the-scenes changes to simplify how we build rdme for GitHub Actions users — no actual customer impact here. quick breakdown of the changes:

  • replaces @rollup/plugin-terser with rollup-plugin-esbuild for minifying our GitHub Actions code. the latter accomplishes basically the same result as the former, but in a tiny fraction of the time.
  • we previously forked our GHA build process into a development vs. production setup since terser takes so long. since rollup-plugin-esbuild adds a negligible amount of time to our build process, there's no need to have two setups. this PR combines them into a single build process.
  • added a bunch of steps to our CI to further assert that the GitHub Actions build is what we expect.

🧬 QA & Testing

does CI still pass?

@kanadgupta kanadgupta added refactor Issues about tackling technical debt GHA / CI Issues specific to GitHub Actions or other CI environments enhancement New feature or request labels Feb 27, 2025
@kanadgupta kanadgupta marked this pull request as ready for review February 27, 2025 01:00
rdme: help

- name: Assert that previous step failed
if: ${{ steps.help-fail.outcome == 'failure' }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

neat

@@ -1,11 +1,10 @@
// @ts-check
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can this file be TS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it requires installing a whole add'l dependency so i opted not to: https://rollupjs.org/command-line-interface/#config-intellisense

@kanadgupta kanadgupta merged commit cd73667 into next Feb 27, 2025
10 checks passed
@kanadgupta kanadgupta deleted the kanad-2025-02-19/esbuild-instead-of-terser branch February 27, 2025 23:32
@kanadgupta kanadgupta added the needs-backport-to-v9 PRs that need to backported to the 9.x channel label Feb 27, 2025
kanadgupta added a commit that referenced this pull request Feb 28, 2025
## 🧰 Changes

[node v20.18.3](https://nodejs.org/en/blog/release/v20.18.3) officially
marked import attributes [as
stable](https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes).
we have a lot of weird workarounds with our `oclif` setup in this repo
in an effort to avoid import attributes and the ugly
`ExperimentalWarning` outputs we see, but i don't think those should be
a concern anymore, for a few reasons:

- our minimum required node.js version is node 20, so it should be a
non-disruptive change for users to upgrade to the latest node 20 channel
- even if folks are on <20.18.2, the `ExperimentalWarning` outputs log
to `stderr` so it shouldn't affect any command output-based workflows,
which almost always are relying on `stdout`. plus, we discourage this
kind of scripting and recommend folks use exit codes instead

given the above, this PR brings back JSON imports of `package.json` and
vastly simplifies a lot of weirdness in this codebase.

## 🧬 QA & Testing

we've added a lot of good test coverage now and i've been playing around
with the CLI and github actions build locally and everything seems to
work as expected.

very unlikely we'll run into the fiasco in
#1117, especially now with the
cleanups in this PR and #1172.
@kanadgupta
Copy link
Copy Markdown
Contributor Author

🎉 This PR is included in version 10.2.0-next.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

kanadgupta added a commit that referenced this pull request Mar 19, 2025
a few small behind-the-scenes changes to simplify how we build `rdme`
for GitHub Actions users — no actual customer impact here. quick
breakdown of the changes:

- [x] replaces `@rollup/plugin-terser` with `rollup-plugin-esbuild` for
minifying our GitHub Actions code. the latter accomplishes basically the
same result as the former, but in a tiny fraction of the time.
- [x] we previously forked our GHA build process into a development vs.
production setup since terser takes so long. since
`rollup-plugin-esbuild` adds a negligible amount of time to our build
process, there's no need to have two setups. this PR combines them into
a single build process.
- [x] added a bunch of steps to our CI to further assert that the GitHub
Actions build is what we expect.

does CI still pass?
kanadgupta added a commit that referenced this pull request Mar 19, 2025
## 🧰 Changes

[node v20.18.3](https://nodejs.org/en/blog/release/v20.18.3) officially
marked import attributes [as
stable](https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes).
we have a lot of weird workarounds with our `oclif` setup in this repo
in an effort to avoid import attributes and the ugly
`ExperimentalWarning` outputs we see, but i don't think those should be
a concern anymore, for a few reasons:

- our minimum required node.js version is node 20, so it should be a
non-disruptive change for users to upgrade to the latest node 20 channel
- even if folks are on <20.18.2, the `ExperimentalWarning` outputs log
to `stderr` so it shouldn't affect any command output-based workflows,
which almost always are relying on `stdout`. plus, we discourage this
kind of scripting and recommend folks use exit codes instead

given the above, this PR brings back JSON imports of `package.json` and
vastly simplifies a lot of weirdness in this codebase.

## 🧬 QA & Testing

we've added a lot of good test coverage now and i've been playing around
with the CLI and github actions build locally and everything seems to
work as expected.

very unlikely we'll run into the fiasco in
#1117, especially now with the
cleanups in this PR and #1172.
kanadgupta added a commit that referenced this pull request Mar 19, 2025
a few small behind-the-scenes changes to simplify how we build `rdme`
for GitHub Actions users — no actual customer impact here. quick
breakdown of the changes:

- [x] replaces `@rollup/plugin-terser` with `rollup-plugin-esbuild` for
minifying our GitHub Actions code. the latter accomplishes basically the
same result as the former, but in a tiny fraction of the time.
- [x] we previously forked our GHA build process into a development vs.
production setup since terser takes so long. since
`rollup-plugin-esbuild` adds a negligible amount of time to our build
process, there's no need to have two setups. this PR combines them into
a single build process.
- [x] added a bunch of steps to our CI to further assert that the GitHub
Actions build is what we expect.

does CI still pass?
kanadgupta added a commit that referenced this pull request Mar 19, 2025
## 🧰 Changes

[node v20.18.3](https://nodejs.org/en/blog/release/v20.18.3) officially
marked import attributes [as
stable](https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes).
we have a lot of weird workarounds with our `oclif` setup in this repo
in an effort to avoid import attributes and the ugly
`ExperimentalWarning` outputs we see, but i don't think those should be
a concern anymore, for a few reasons:

- our minimum required node.js version is node 20, so it should be a
non-disruptive change for users to upgrade to the latest node 20 channel
- even if folks are on <20.18.2, the `ExperimentalWarning` outputs log
to `stderr` so it shouldn't affect any command output-based workflows,
which almost always are relying on `stdout`. plus, we discourage this
kind of scripting and recommend folks use exit codes instead

given the above, this PR brings back JSON imports of `package.json` and
vastly simplifies a lot of weirdness in this codebase.

## 🧬 QA & Testing

we've added a lot of good test coverage now and i've been playing around
with the CLI and github actions build locally and everything seems to
work as expected.

very unlikely we'll run into the fiasco in
#1117, especially now with the
cleanups in this PR and #1172.
@kanadgupta kanadgupta added backported-to-v9 PRs that have been backported to the 9.x channel and removed needs-backport-to-v9 PRs that need to backported to the 9.x channel labels Mar 19, 2025
@kanadgupta
Copy link
Copy Markdown
Contributor Author

🎉 This PR is included in version 10.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported-to-v9 PRs that have been backported to the 9.x channel enhancement New feature or request GHA / CI Issues specific to GitHub Actions or other CI environments refactor Issues about tackling technical debt released on @next released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants