Skip to content

Conversation

@lwasser
Copy link
Member

@lwasser lwasser commented Jan 24, 2026

addresses (in part) #385

What:

CIRCLE ci is currently failing but also we cant run tests. so here is a fix - move to GHA - and once we have npm permissions we can migrate the deploy here too.

We also should consider whether semantic release is ideal. Most contributors don't think about commit messages. In a perfect world, it's great. But we don't live in a perfect world. So i prefer a changelog that we just ask people to fill out for every pr. Its a small ask.

But let's talk about this in a separate issue and get tests running first.
Why:

Tests don't run now

How:

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@lwasser
Copy link
Member Author

lwasser commented Jan 24, 2026

Ok - this isn't perfect, BUT it runs the tests so we can see as we update deps what breaks and what works.

runs-on: ubuntu-latest
needs: validate
# Disabled until npm permissions are set up
if: false && github.ref == 'refs/heads/main' && github.event_name == 'push'
Copy link
Member Author

Choose a reason for hiding this comment

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

I've intentionally turned this off. I suggest we consider rather than continuous deployment a safer approach where we deploy on release only

We can then protect the environment here, so only a few maintainers - I suggest we start with @JoshuaKGoldberg, @JimMadge, and me - to begin have access to releasing. I think this is the most secure route.

Continuous deployment, to me, sounds insecure and dangerous. I know at least a few PRs have been submitted here that I closed as ill-intentioned. We should lock things down as much as possible.


### Pre-commit hooks

We've set up Husky to run `kcd-scripts pre-commit` automatically before each
Copy link
Member Author

Choose a reason for hiding this comment

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

This is neat - I'm still learning about Husky, but we use pre-commit everywhere in the Python repos I work on!

@@ -0,0 +1,30 @@
name: Lint
Copy link
Member Author

Choose a reason for hiding this comment

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

Break out linting into it's own build so we can quickly catch when linting has not been applied!

@lwasser lwasser changed the title chore(ci): run tests on gha chore(ci): move builds test suite, linting and deploy to gha Jan 24, 2026
@lwasser lwasser enabled auto-merge (squash) January 24, 2026 21:12
@lwasser
Copy link
Member Author

lwasser commented Jan 24, 2026

oops - this is breaking now - converting to draft... it started to break when i rebased....

lwasser and others added 2 commits January 24, 2026 14:33
chore(ci): break out testing
…n tests aren't running in ci. (all-contributors#399)

* chore(deps): bump yargs from 15.4.1 to 18.0.0

Bumps [yargs](https://github.com/yargs/yargs) from 15.4.1 to 18.0.0.
- [Release notes](https://github.com/yargs/yargs/releases)
- [Changelog](https://github.com/yargs/yargs/blob/main/CHANGELOG.md)
- [Commits](yargs/yargs@v15.4.1...v18.0.0)

---
updated-dependencies:
- dependency-name: yargs
  dependency-version: 18.0.0
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump prettier from 2.8.8 to 3.8.1

Bumps [prettier](https://github.com/prettier/prettier) from 2.8.8 to 3.8.1.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@2.8.8...3.8.1)

---
updated-dependencies:
- dependency-name: prettier
  dependency-version: 3.8.1
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore(deps): bump chalk from 4.1.2 to 5.6.2

Bumps [chalk](https://github.com/chalk/chalk) from 4.1.2 to 5.6.2.
- [Release notes](https://github.com/chalk/chalk/releases)
- [Commits](chalk/chalk@v4.1.2...v5.6.2)

---
updated-dependencies:
- dependency-name: chalk
  dependency-version: 5.6.2
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* chore: update gitignore

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@lwasser lwasser removed the request for review from JoshuaKGoldberg January 24, 2026 21:57
@lwasser lwasser disabled auto-merge January 24, 2026 21:57
@lwasser lwasser marked this pull request as draft January 24, 2026 21:57
@lwasser
Copy link
Member Author

lwasser commented Jan 24, 2026

Ok i need some help here. Essentially, I'm running into a lot of dependency issues as we update, and tests are breaking. This is why I wanted the tests in place before updating deps, but also.. Now everything is conflicting and breaking. If anyone is able to help i'd greatly appreciate!

@lwasser lwasser changed the title chore(ci): move builds test suite, linting and deploy to gha HELP (dependency issues) chore(ci): move builds test suite, linting and deploy to gha Jan 24, 2026
@jdalrymple
Copy link
Contributor

lollzzz the last commit 😂

@jdalrymple
Copy link
Contributor

Looking now 👁️ 👁️ !!

@jdalrymple
Copy link
Contributor

Looks like what i ran into before! I know how to sort this 🫡

@jdalrymple
Copy link
Contributor

Got it, ill push up the changes!

@jdalrymple
Copy link
Contributor

Ill probs push the specific changes to a new PR since its a bit. It broke due to the prettier dependency update, since they changed their API.

@lwasser lwasser changed the title HELP (dependency issues) chore(ci): move builds test suite, linting and deploy to gha chore(ci): move builds test suite, linting and deploy to gha Jan 24, 2026
@lwasser
Copy link
Member Author

lwasser commented Jan 24, 2026

Ill probs push the specific changes to a new PR since its a bit. It broke due to the prettier dependency update, since they changed their API.

Thank you!! As i was just walking I realized that we should probably merge this PR just for the ci part - nothing else. The JEST snapshot updates (I didn't understand what your changes were, and now I know!!) I think are fine!

and i opened #400 i suspect kcd scripts is a problem because it's a few years old and so it's calling older dependencies (i think).

i'll watch for your pr and will rebase !!

@lwasser lwasser marked this pull request as ready for review January 24, 2026 23:46
@lwasser lwasser marked this pull request as draft January 25, 2026 00:38
@jdalrymple
Copy link
Contributor

jdalrymple commented Jan 25, 2026

Ill probs push the specific changes to a new PR since its a bit. It broke due to the prettier dependency update, since they changed their API.

Thank you!! As i was just walking I realized that we should probably merge this PR just for the ci part - nothing else. The JEST snapshot updates (I didn't understand what your changes were, and now I know!!) I think are fine!

and i opened #400 i suspect kcd scripts is a problem because it's a few years old and so it's calling older dependencies (i think).

i'll watch for your pr and will rebase !!

Yea i saw that as well in my OG PR, but figured that would be too deep into the scope creep 😂

I pushed up a branch to review #403 - tldr; updated references to the prettier functions that are now async in v3, the kcd scripts pass locally, but the gha script currently available doesnt run that formatter, just the linter as far as i can see. I left the PR as a draft so you can see what was done. Feel free to add merge it, at least that will bring things in line with the prettier changes

@lwasser
Copy link
Member Author

lwasser commented Jan 31, 2026

Ok, this pr i may just close and start over, given we have decided to break up all of the tasks (which I think is best!!) Closing this and i'll focus on building the package next IF no one else is working on that.

@lwasser lwasser closed this Jan 31, 2026
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