Fix lint and add lint + prettier to CI #461
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZhangYiJiang
commented
Dec 11, 2024
| @@ -4,7 +4,7 @@ module.exports = { | |||
| es2021: true, | |||
| }, | |||
| root: true, | |||
| extends: ['standard-with-typescript', 'plugin:react/recommended'], | |||
There was a problem hiding this comment.
This plugin only appears to be used in app, so configuring it here seems to be a mistake
| @@ -211,7 +211,9 @@ export class HttpCallNodeImpl extends NodeImpl<HttpCallNode> { | |||
| const method = getInputOrData(this.data, inputs, 'method', 'string'); | |||
| const url = getInputOrData(this.data, inputs, 'url', 'string'); | |||
|
|
|||
| // TODO: Use URL.canParse when we drop support for Node 18 | |||
There was a problem hiding this comment.
Not sure if we have an official Node support policy
ZhangYiJiang
commented
Dec 11, 2024
| @@ -74,6 +74,7 @@ export type ChatCompletionChunk = { | |||
| model: string; | |||
| }; | |||
|
|
|||
| /* eslint-disable @typescript-eslint/naming-convention */ | |||
There was a problem hiding this comment.
Not sure if it is better to leave this to match API naming convention or rename to match Rivet/JS naming convention
|
The new checks thingy is pretty nice although it does mean the existing warnings will appear on every PR until they are resolved (but it also means new warnings will show up more prominently which is nice) |
|
I'll merge this for now so we can also run lint on the new PRs coming in, the warnings should be fixed later |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've noticed while there is a lint and prettier command configured, they are not actually enforced by CI, so some of the code has drifted. Most can be automatically or manually fixed but a few places where the linter has picked up genuine bugs has been left due to lack of context needed to fix them.
This PR tries to bring the code up to lint as much as possible conservatively and add them to CI so they don't drift in the future.
The one big bug that it found is an import cycle between RunGraphNode and the globalRivetNodeRegistry global. There's no easy way to reconcile this other than making the global registry lazy by putting it behind a getter, but that's more code alteration than seems good for a lint fix PR so I'll delegate that to a future PR to fix.