-
Notifications
You must be signed in to change notification settings - Fork 32
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
Uplift internal linting #1370
Uplift internal linting #1370
Conversation
🦋 Changeset detectedLatest commit: 6a10c00 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Description makes sense 👍. Just opining on the TODOs, I'll take a more detailed look later.
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.
Bigly effort, ty 🙌
description: | ||
'Add empty exports to Jest files for compliance with TypeScript isolated modules', |
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.
Thoughts on us having documentation (e.g. another page similar to https://seek-oss.github.io/skuba/CHANGELOG.html) that provides more context on each patch that we can link from the CLI?
Not for this PR, but I'm happy to take a look at this if we agree it's a decent idea. Not sure if it would be overkill for the average patch, though we tend to write detailed explanations already that end up buried in the changelog via Changesets.
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.
Yeah that could be good. Maybe if we give them IDs/names we could also later give a way to opt out of a change (e.g. node 20 later)
Co-authored-by: Ryan Ling <ryan@outlook.com.au>
Co-authored-by: Ryan Ling <ryan@outlook.com.au>
Co-authored-by: Ryan Ling <ryan@outlook.com.au>
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.
🚀
Thanks, you're very good at copyediting :) |
Replacing #1346, but same motivation:
Goal is:
I'm still newish to this codebase, so there may be crimes here, and my manual and automated testing is somewhat limited, so things to look out for tests to add (or run) would be appreciated.