Skip to content

Split JavaScript and Ruby CI workflows - #1273

Open
joshblack with Copilot wants to merge 16 commits into
mainfrom
copilot/update-workflows-separate-ruby-js
Open

Split JavaScript and Ruby CI workflows#1273
joshblack with Copilot wants to merge 16 commits into
mainfrom
copilot/update-workflows-separate-ruby-js

Conversation

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Separate JavaScript and Ruby CI into independent workflows with dedicated lint and test jobs covering every package.

  • JavaScript
    • Runs lint, build, tests, type checks, and package validation across all npm packages.
    • Updates the styled package test tooling so its suite runs under the workspace dependency model.
  • Ruby
    • Runs lint across all gems.
    • Builds Octicons assets and tests the base, helper, and Jekyll gems against the generated package.
  • Security
    • Restricts workflow token permissions to read-only repository contents.

@changeset-bot

changeset-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 20fac0b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@joshblack joshblack left a comment

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.

@copilot bias to lowercase for names (e.g. use lint and don't rename to name: Lint). Update the JavaScript workflow name to just be js. Remove changes unrelated to the workflow updates (this should only change GitHub actions, no longer files)

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot bias to lowercase for names (e.g. use lint and don't rename to name: Lint). Update the JavaScript workflow name to just be js. Remove changes unrelated to the workflow updates (this should on...

Addressed in 3c8e857: workflow names are lowercase (js/ruby), redundant capitalized job names are removed, and all non-Actions files are restored.

Copilot AI requested a review from joshblack July 27, 2026 19:36
@joshblack joshblack added the skip changeset This change does not need a changelog label Jul 27, 2026
@joshblack

Copy link
Copy Markdown
Member

@copilot fix CI

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix CI

Fixed in cdbfbf2. The JavaScript workflow now restores the shared build artifact and isolates workspace installs using the previously passing CI structure. All matrix checks pass locally; new Actions runs are awaiting approval.

@joshblack joshblack left a comment

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.

@copilot refactor the js workflows where possible to use the npm cli option for running a command across all workspaces instead of doing the matrix strategy

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot refactor the js workflows where possible to use the npm cli option for running a command across all workspaces instead of doing the matrix strategy

Addressed in 3dc3d42. The JavaScript lint and test matrices are replaced with npm workspace commands while preserving the passing test coverage.

Copilot AI requested a review from joshblack July 27, 2026 21:38

@joshblack joshblack left a comment

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.

@copilot don't use the matrix strategy for ruby either, just run lint as one step that covers everything. For js tests, the syntax seems strange to target specific workspaces. Let's just run tests and lint and what not as one step (or tell me why this wouldn't work)

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot apply review feedback

Applied in the latest commit:

  1. ruby.yml: Node version now uses node-version-file: '.nvmrc' (picks up 26) instead of hardcoding 20.
  2. Rakefile: Gem selection uses max_by { |f| File.mtime(f) } to pick the most recently built gem, with a clear error if none exist.
  3. lib/octicons_jekyll/Gemfile: Version constraint updated from 19.8.0 to 19.9.0 to match the built gem so Bundler will actually use it.

@joshblack

Copy link
Copy Markdown
Member

@copilot fix CI

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix CI

Fixed in the latest commit. The jekyll-octicons.gemspec still declared octicons 19.8.0 while the Gemfile was updated to 19.9.0 — Bundler couldn't resolve the conflicting constraints. Updated the gemspec to 19.9.0 so both are consistent.

@joshblack

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in the merge commit. Conflicts were:

  • .github/workflows/ci.yml — kept deleted (replaced by the new javascript.yml/ruby.yml workflows)
  • lib/octicons_jekyll/Gemfile — took main's 19.32.0
  • lib/octicons_jekyll/jekyll-octicons.gemspec — took main's 19.32.0

@joshblack joshblack left a comment

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.

@copilot update these workflows so they are pinned similar to others in the project now (e.g. they use a sha but have the version as a comment afterwards). make sure they are using the latest versions of actions, too

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot update these workflows so they are pinned similar to others in the project now (e.g. they use a sha but have the version as a comment afterwards). make sure they are using the latest versions...

Done in the latest commit. All actions in both workflows are now pinned to the same SHAs used elsewhere in the project, with the version as an inline comment:

  • actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
  • actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
  • actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
  • actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
  • ruby/setup-ruby@95ef2b042f9d7a56d8268cba8559e2842e2ad01b # v1.321.0

Copilot AI requested a review from joshblack July 29, 2026 18:02
Comment thread package.json Outdated
"build": "script/build.js --input icons/**/*.svg --output lib/build/data.json",
"changeset:version": "changeset version && script/version lib/octicons_node/package.json",
"lint": "eslint tests",
"lint": "eslint tests lib/octicons_node/index.js lib/octicons_node/tests lib/octicons_react/pages lib/octicons_react/script lib/octicons_react/src lib/octicons_styled/script lib/octicons_styled/src",

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.

@copilot could we update this so it just checks all js/ts/tsx files?

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.

Done in the latest commit — the lint script now runs eslint '**/*.{js,ts,tsx}'.

Copilot AI requested a review from joshblack July 29, 2026 18:06
@joshblack

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

…ows-separate-ruby-js

# Conflicts:
#	package.json

Copilot AI commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in the merge commit. Conflicts were:

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

Labels

skip changeset This change does not need a changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants