fix(lint): Upgrade ESLint #704
Conversation
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
had a problem deploying
to
fix-eslint
GitHub Actions
Failure
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
had a problem deploying
to
fix-eslint
GitHub Actions
Failure
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint-kibana
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint-app
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
had a problem deploying
to
fix-eslint
GitHub Actions
Failure
asharonbaltazar
had a problem deploying
to
fix-eslint
GitHub Actions
Failure
asharonbaltazar
had a problem deploying
to
fix-eslint
GitHub Actions
Failure
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
| delete process.env.attachmentsBucketName; | ||
|
|
||
| expect(() => handler({} as APIGatewayEvent)).toThrowError; |
There was a problem hiding this comment.
This test never ran as it was never called – eslint caught the error and I fixed it
There was a problem hiding this comment.
handler can't have thrown an error because it was being mocked, so we had to spy on the response function like we did in the previous tests
| validateEnvVariable("attachmentsBucketName"); | ||
| validateEnvVariable("attachmentsBucketRegion"); | ||
|
|
There was a problem hiding this comment.
I moved these into the try so the catch block catches them
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint-kibana
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint-app
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
| @@ -86,6 +86,6 @@ describe("Kafka producer functions", () => { | |||
|
|
|||
| it("should throw an error if brokerString is not defined", () => { | |||
| delete process.env.brokerString; | |||
| expect(() => getProducer()).toThrowError; | |||
| expect(() => getProducer()).toThrowError(); | |||
There was a problem hiding this comment.
Test never ran so it wasn't called
| @@ -63,7 +63,7 @@ export const downloadAVDefinitions = async (): Promise<void[]> => { | |||
| // Download each file in the bucket | |||
| const downloadPromises: Promise<void>[] = definitionFileKeys.map( | |||
| (filenameToDownload) => { | |||
| return new Promise<void>(async (resolve, reject) => { | |||
| return new Promise<void>((resolve, reject) => { | |||
There was a problem hiding this comment.
This is the async/await version of the Promise constructor antipattern!
Never ever use an async function as a Promise executor function
| submitterName: | ||
| `${user?.user?.given_name} ${user?.user?.family_name}` ?? "N/A", |
There was a problem hiding this comment.
The fallback would never run since the first value is a string. I'm gonna change the check so it falls back if the user object is empty
There was a problem hiding this comment.
True. Yeah, because nullish operator only checks for null or undefined. Wondering if we should flip to a || instead or just remove?
There was a problem hiding this comment.
Resolved with fe8de8b
| const stopCountdown = useCallback(() => { | ||
| setIsCountdownRunning(false); | ||
| }; | ||
| }, []); | ||
|
|
||
| // Will set running false and reset the seconds to initial value | ||
| const resetCountdown = useCallback(() => { | ||
| stopCountdown(); | ||
| setCount(minutesToCountDown); | ||
| }, [stopCountdown]); | ||
| }, [stopCountdown, minutesToCountDown]); |
There was a problem hiding this comment.
Yay to the new tests to confirm these still work!
react-app/src/vite-env.d.ts
Outdated
There was a problem hiding this comment.
Fancy new types file for env variables
| @@ -3,7 +3,7 @@ import { raiIssueSchema } from "../../.."; | |||
| export const transform = (id: string) => { | |||
| return raiIssueSchema.transform((data) => ({ | |||
| id, | |||
| makoChangedDate: !!data.timestamp | |||
| makoChangedDate: data.timestamp | |||
There was a problem hiding this comment.
These concern me only because I don't know if I can trust the type here enough to remove it. If we are able to confirm that the code sufficiently validates the type is what we say it is I am okay with this
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
|
Code Climate has analyzed commit fe8de8b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 37.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 53.4% (0.0% change). View more on Code Climate. |
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
asharonbaltazar
temporarily deployed
to
fix-eslint
GitHub Actions
Inactive
|
I'
Yeah, I did some thinking. We're about to do a massive overhaul of how this sinking and stuff works anyway. Feel free to merge Asharon |
|
🎉 This PR is included in version 1.5.0-val.71 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
Upgrading to ESLint v9 broke our linting command since it no longer recognized the old config file.
Linked Issues to Close
https://qmacbis.atlassian.net/browse/OY2-29548
Approach
libandbin/clias well