Miscellaneous cleanup #14
Conversation
khawkins
commented
Jun 6, 2024
khawkins
commented
- Renamed some files to follow naming conventions.
- Consolidated some helper functions.
- Updated rule messages and descriptions.
- Renamed some files to follow naming conventions. - Consolidated some helper functions. - Updated rule messages and descriptions.
khawkins
requested review from
sfdctaka,
maliroteh-sf,
ben-zhang-at-salesforce and
haifeng-li-at-salesforce
as code owners
|
Everything looks pretty good over all. I did a few cleanup things as mentioned in the description. I'll go through and highlight a couple in the details. |
| @@ -0,0 +1,8 @@ | |||
| { | |||
There was a problem hiding this comment.
Figured we may as well recommend the ESLint and Pretter extensions, since we make use of both.
| @@ -0,0 +1,3 @@ | |||
| { | |||
| "editor.defaultFormatter": "esbenp.prettier-vscode" | |||
There was a problem hiding this comment.
Did this just so any potential formatter conflict configured locally wouldn't wreak havoc when formatting in VSCode. Prettier will automatically read its configuration from its standard locations in the project, so we don't need to configure it here, more than just saying that this is the formatter we use.
| @@ -14,7 +14,7 @@ import { NO_SEMI_ANTI_JOIN_SUPPORTED_RULE_ID } from '../rules/graphql/no-semi-an | |||
| import { NO_MORE_THAN_1_PARENT_RECORD_RULE_ID } from '../rules/graphql/no-more-than-1-parent-record.js'; | |||
| import { NO_MORE_THAN_3_CHILD_ENTITIES_RULE_ID } from '../rules/graphql/no-more-than-3-child-entities.js'; | |||
| import { NO_MORE_THAN_3_ROOT_ENTITIES_RULE_ID } from '../rules/graphql/no-more-than-3-root-entities.js'; | |||
| import { createScopedModuleRuleName } from '../util/createScopedModuleRuleName.js'; | |||
| import { createScopedModuleRuleName } from '../util/rule-helpers.js'; | |||
There was a problem hiding this comment.
I consolidated these into an aggregate helper utility module, just because it didn't seem like the ones I consolidated, needed to have their own separate space.
There was a problem hiding this comment.
good idea moving createRule, createScopedModuleRuleName into graphql-ast-utils.ts.
| import { getClosestAncestorByType } from '../../util/graphqlAstUtils'; | ||
| import { GraphQLESTreeNode } from './types'; | ||
| import { getDocUrl } from '../../util/rule-helpers'; | ||
| import { getClosestAncestorByType } from '../../util/graphql-ast-utils'; |
There was a problem hiding this comment.
Also renamed a couple of modules/files like this, just for more consistent TypeScript naming conventions.
| import { GraphQLESTreeNode } from './types'; | ||
| import { getDocUrl } from '../../util/rule-helpers'; | ||
| import { getClosestAncestorByType } from '../../util/graphql-ast-utils'; | ||
| import { GraphQLESTreeNode } from '../../util/types'; |
There was a problem hiding this comment.
I moved everything that wasn't explicitly a rule, out of the rules/ tree. It's good to keep the rules tree dedicated to rules, so you can see them all at a glance.
| @@ -1,3 +1,27 @@ | |||
| /* | |||
There was a problem hiding this comment.
Just to be safe, I added the full copy/paste of the license as worded in the original project. I want to make sure we give proper attribution.
I think there's a good chance that if we can properly tweak our project configuration, we'll be able to remove this file in favor of referencing it from the original project. 🤞
There was a problem hiding this comment.
we are baffled when seeing mousing over shows the type correctly, but has wiggling line. will spend more fiddling around project setting to see if we can cure this dark spot.
test/rules/apex/apex-import.ts
Outdated
There was a problem hiding this comment.
nitpick: should we rename this file apex-import.ts to apex-import.spec.ts to keep the name consistent with other tests?
There was a problem hiding this comment.
Yeah that's a good idea. I'll rename it.
