-
Notifications
You must be signed in to change notification settings - Fork 147
chore(ci): move builds test suite, linting and deploy to gha #397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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' |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
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!
|
oops - this is breaking now - converting to draft... it started to break when i rebased.... |
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>
|
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! |
|
lollzzz the last commit 😂 |
|
Looking now 👁️ 👁️ !! |
|
Looks like what i ran into before! I know how to sort this 🫡 |
|
Got it, ill push up the changes! |
|
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 |
|
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. |
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: