Configure code style #98
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
`npm audit fix --force` note: possibly destructive, roll back if necessary 10 vulnerabilities remaining (3 moderate, 7 high)
…into configure-code-style
kbighorse
commented
Oct 25, 2023
kbighorse
commented
Oct 25, 2023
|
(Unsubscribing from this to prevent email noise on new commits — re-ping me if I need to look at it) |
-- What This rule (https://eslint.org/docs/latest/rules/no-param-reassign) specifies that you cannot reassign a variable that is passed into a function as a parameter (good, makes sense), but for some reason it also says that you cannot alter properties of an incoming variable if it's an object (bad, doesn't make sense). Updating the eslintrc file to account for this
Also updating the airbnb base linter config version
kbighorse
commented
Oct 26, 2023
Also adding nvmrc for consistent node versioning
kbighorse
commented
Oct 27, 2023
Seeing if this helps with websocket errors
-- What We do not need to specify the extensions in the command line for the eslint runner (if we need specific extensions only in the future, we can add these into the config file where they belong). Additionally, found a bad situation where eslint was reading from the site build directory if a local build it present, which resulted in tens of thousands of errors as a result of transpiled code from external libraries. Have added config values to ignore the build directory.
kbighorse
commented
Oct 30, 2023
echappen
requested changes
Oct 31, 2023
There was a problem hiding this comment.
Thank you both for your work on this! I have a few blocking questions about where dev dependencies are being loaded. Could we also add some written documentation about our linting strategy (why we're doing it, what rules we're using, when in the pipeline it runs, etc).
-- What Linter has identified that certain imports should be listed in dependencies rather than devDependencies
|
@echappen I think I've addressed each of your concerns. I've added some documentation about prettier, as well as our own template precommit hook. The linter forced me to move certain dependencies back to production, but I think that makes sense. Let me know if there's anything else that looks off. Thanks! |
|
great work all! |
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.
Changes proposed in this pull request:
security considerations
[Note the any security considerations here, or make note of why there are none]