-
Notifications
You must be signed in to change notification settings - Fork 25
EVG-20048: Introduce check-file plugin #2109
Changes from 22 commits
b1ba409
a3e406c
9889520
647483b
d54ce57
87b0510
f43cd07
4976741
bd90447
f8ea637
aa95bc4
0848548
bc5a909
90c44fe
802787e
1f66210
c1a86a6
c8d17f9
ad6d4c0
59e8c90
9034003
de7196a
827ea8e
750a352
161996b
916dcad
998cf28
1013516
8fc1528
bbece55
33f1d7b
45a7c6e
5defa1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ module.exports = { | |
"plugin:jsdoc/recommended-typescript-error", | ||
"plugin:prettier/recommended", // Note: prettier must ALWAYS be the last extension. | ||
], | ||
plugins: ["@typescript-eslint", "sort-destructure-keys"], | ||
plugins: ["@typescript-eslint", "sort-destructure-keys", "check-file"], | ||
settings: { | ||
react: { | ||
version: "detect", | ||
|
@@ -96,6 +96,28 @@ module.exports = { | |
// Rules for prettier. | ||
"prettier/prettier": errorIfStrict, // Makes Prettier issues warnings rather than errors. | ||
"sort-destructure-keys/sort-destructure-keys": errorIfStrict, | ||
"check-file/filename-naming-convention": [ | ||
errorIfStrict, | ||
{ | ||
// GraphQL fragments, mutations and queries | ||
"src/gql/fragments/**/*.graphql": "CAMEL_CASE", | ||
"src/gql/(mutations,queries)/**/*.graphql": "KEBAB_CASE", | ||
// Cypress | ||
"cypress/integration/**/*.ts": "SNAKE_CASE", | ||
// Scripts | ||
"scripts/**/*.{js,ts}": "KEBAB_CASE", | ||
// JS and TS with exceptions | ||
"src/**/!(vite-env.d|custom-queries)*.{js,ts}": "CAMEL_CASE", | ||
// All tsx with exceptions | ||
"src/**/!(use|getFormSchema|index|test-utils|toast-decorator|schemaFields|getColumnsTemplate|githubPRLinkify|jiraLinkify)*.tsx": | ||
"PASCAL_CASE", | ||
// tsx exceptions | ||
"src/**/(use|getFormSchema|index)*.tsx": "CAMEL_CASE", | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we further trim these down? So we don't have 4 different conventions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the smallest set that makes sense to me. Let me know what you want to trim and why. |
||
{ | ||
ignoreMiddleExtensions: true, | ||
}, | ||
], | ||
}, | ||
overrides: [ | ||
// For React Typescript files in src. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { Divider } from "./Divider"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file should be lower case and doesn't need the TSX extension. |
||
import { FiltersWrapper } from "./filters"; | ||
import { inactiveElementStyle } from "./Inactive"; | ||
import { inactiveElementStyle } from "./inactive"; | ||
import { | ||
ErrorMessage, | ||
InputLabel, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,22 +2,15 @@ import { useMemo } from "react"; | |
import { useQuery } from "@apollo/client"; | ||
import styled from "@emotion/styled"; | ||
import { LeafyGreenTableRow } from "@leafygreen-ui/table/new"; | ||
import { | ||
getCommitsRoute, | ||
getPatchRoute, | ||
getTaskRoute, | ||
getVersionRoute, | ||
} from "constants/routes"; | ||
import { size } from "constants/tokens"; | ||
import { convertFamilyTrigger } from "constants/triggers"; | ||
import { | ||
GeneralSubscription, | ||
Selector, | ||
UserSubscriptionsQuery, | ||
UserSubscriptionsQueryVariables, | ||
GeneralSubscription, | ||
Selector, | ||
} from "gql/generated/types"; | ||
import { USER_SUBSCRIPTIONS } from "gql/queries"; | ||
import { ResourceType } from "types/triggers"; | ||
|
||
export const useSubscriptionData = () => { | ||
const { data } = useQuery< | ||
|
@@ -108,34 +101,7 @@ const ExpandedBlock = styled.pre` | |
padding: ${size.s} ${size.l}; | ||
`; | ||
|
||
export const getResourceRoute = ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this function so this file exports the hook only. |
||
resourceType: ResourceType, | ||
selector: Selector | ||
) => { | ||
const { data: id, type } = selector; | ||
|
||
if (!id) { | ||
return ""; | ||
} | ||
|
||
switch (resourceType) { | ||
case ResourceType.Build: | ||
case ResourceType.Version: { | ||
if (type === "project") { | ||
return getCommitsRoute(id); | ||
} | ||
return getVersionRoute(id); | ||
} | ||
case ResourceType.Patch: | ||
return getPatchRoute(id, { configure: false }); | ||
case ResourceType.Task: | ||
return getTaskRoute(id); | ||
default: | ||
return ""; | ||
} | ||
}; | ||
|
||
export const formatRegexSelectors = (regexSelectors: Selector[]) => ({ | ||
const formatRegexSelectors = (regexSelectors: Selector[]) => ({ | ||
"regex-selectors": regexSelectors.reduce<Record<string, string>>( | ||
(obj, { data, type }) => ({ | ||
...obj, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { | ||
getCommitsRoute, | ||
getPatchRoute, | ||
getTaskRoute, | ||
getVersionRoute, | ||
} from "constants/routes"; | ||
import { Selector } from "gql/generated/types"; | ||
import { ResourceType } from "types/triggers"; | ||
|
||
export const getResourceRoute = ( | ||
resourceType: ResourceType, | ||
selector: Selector | ||
) => { | ||
const { data: id, type } = selector; | ||
|
||
if (!id) { | ||
return ""; | ||
} | ||
|
||
switch (resourceType) { | ||
case ResourceType.Build: | ||
case ResourceType.Version: { | ||
if (type === "project") { | ||
return getCommitsRoute(id); | ||
} | ||
return getVersionRoute(id); | ||
} | ||
case ResourceType.Patch: | ||
return getPatchRoute(id, { configure: false }); | ||
case ResourceType.Task: | ||
return getTaskRoute(id); | ||
default: | ||
return ""; | ||
} | ||
}; |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't actually need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this change is necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we change this back to .ts then I will either need to add an exception since it's currently pascal case, or convert it into camelcase. I prefer converting it into camel case. |
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.
Should we hardcode all of these exceptions or can we update these to match the pattern we have established?
e.g. jiraLinkify -> JiraLinkify.tsx
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 reviewed these and think these makes sense as is because I think pascal case should be used to represent react components only. Although these files use JSX, they aren't really React components.
githubPRLinkify
andjiraLinkify
shouldn't be React components so they are easily composable.use
represents hooksgetFormSchema
sometimes uses JSX but is always destructuredindex
isn't generally capitalizedtest-utils
andtoast-decorator
use JSX but are utils and not componentsgetColumnsTemplate
has the same reasoning asgetFormSchema