-
Notifications
You must be signed in to change notification settings - Fork 397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update eslint rules and refactoring types. #2676
Changes from all commits
a7db11b
e40b4f6
67702be
4a1365c
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 |
---|---|---|
|
@@ -30,15 +30,8 @@ | |
"plugin:@angular-eslint/template/process-inline-templates" | ||
], | ||
"rules": { | ||
"curly": "error", | ||
"unused-imports/no-unused-imports": "error", | ||
"@stylistic/semi": [ | ||
"error", | ||
"always" | ||
], | ||
"@stylistic/quote-props": [ | ||
"warn", | ||
"consistent" | ||
], | ||
"@typescript-eslint/explicit-member-accessibility": [ | ||
"error", | ||
{ | ||
|
@@ -67,15 +60,16 @@ | |
"style": "camelCase" | ||
} | ||
], | ||
"curly": "error", | ||
"@stylistic/semi": "error", | ||
"@stylistic/quote-props": [ | ||
"warn", | ||
"consistent" | ||
], | ||
"@stylistic/comma-dangle": [ | ||
"error", | ||
"always-multiline" | ||
], | ||
"@stylistic/eol-last": [ | ||
"error", | ||
"always" | ||
], | ||
"@stylistic/eol-last": "error", | ||
"@stylistic/no-trailing-spaces": "error", | ||
"@typescript-eslint/no-unused-vars": [ | ||
"error", | ||
|
@@ -90,9 +84,7 @@ | |
{ | ||
"extendDefaults": true, | ||
"types": { | ||
"{}": false, | ||
"Object": false, | ||
"Function": false | ||
"{}": false | ||
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 would ban empty object as well, otherwise, type safety is not guaranteed 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. Yes I want to ban {}. but in this repo there are using many {} type. especially {} used in json rpc related type. this means if I remove {} in this repo, this will be super heavy task(Many typing will needed....). so I focused to ban Object and Function. 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. Reducing 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 understand in terms of being a heavy task, so i approve |
||
} | ||
} | ||
] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,7 +126,7 @@ export class Service extends AbstractService { | |
// this.notify(notification); | ||
} | ||
|
||
public setCurrentComponent(currentPageTitle: string | { languageKey: string, interpolateParams?: Object }, activatedRoute: ActivatedRoute): Promise<Edge> { | ||
public setCurrentComponent(currentPageTitle: string | { languageKey: string, interpolateParams?: {} }, activatedRoute: ActivatedRoute): Promise<Edge> { | ||
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.
|
||
return new Promise((resolve, reject) => { | ||
// Set the currentPageTitle only once per ActivatedRoute | ||
if (this.currentActivatedRoute != activatedRoute) { | ||
|
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.
could you fix formatting, why did you change the order around?
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.
to clear rule cateogry. I commented on this commit.
4a1365c
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.
ok,i agree