Skip to content

Improve linting #2787

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.

Sign up for GitHub

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

Jump to bottom
Merged
mturley merged 5 commits into from
Aug 30, 2019
Merged

Improve linting #2787

mturley merged 5 commits into from
Aug 30, 2019

Conversation

@redallen
Copy link
Contributor

@redallen redallen commented Aug 27, 2019 •
edited
Loading

What:

  • Tweak tslint.json to be nicer
  • Fix autofixable errors using new tslint.json

Closes #2362

cc @mturley

Additional issues:

@patternfly-build
Copy link
Collaborator

patternfly-build commented Aug 27, 2019

PatternFly-React preview: https://patternfly-react-pr-2787.surge.sh

@tlabaj tlabaj requested a review from mturley August 29, 2019 14:58
@redallen redallen force-pushed the chore/formatting branch 2 times, most recently from d73cb41 to 3d246a8 Compare August 29, 2019 19:19
@redallen redallen changed the title Chore/formatting Improve linting Aug 29, 2019
mturley
mturley previously approved these changes Aug 30, 2019
View reviewed changes
Copy link
Collaborator

@mturley mturley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly this looks good, but there's still some weird indentation going on. I'll take a look at the config and come back to it. We can go ahead and merge now if you want, if so I'll address the indentation in another PR.

packages/patternfly-4/react-core/src/components/ApplicationLauncher/ApplicationLauncher.tsx
position: DropdownPosition.left,
onSelect: (_event: any): any => undefined,
onToggle: (_value: boolean): any => undefined,
"className": '',
Copy link
Collaborator

@mturley mturley Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weird, what rule added quotes to all of these defaultProps? I wonder why it didn't do the same to all the others?

packages/patternfly-4/react-core/src/components/Badge/Badge.tsx
isRead = false,
className = '',
children = '',
...props }: BadgeProps) => (
Copy link
Collaborator

@mturley mturley Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's still some weird spacing going on here.

packages/patternfly-4/react-core/src/components/Card/CardHeader.tsx
export interface CardHeaderProps extends React.HTMLProps<HTMLDivElement> {
/** Content rendered inside the Card Footer */
children?: React.ReactNode;
children?: React.ReactNode;
Copy link
Collaborator

@mturley mturley Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's interesting that TSLint is ok with 4-space indentation here and 2-space indentation everywhere else. Maybe that's just something it can't automatically --fix?

packages/patternfly-4/react-core/src/components/ChipGroup/ChipButton.tsx
}: ChipButtonProps) => {
return (
<Button variant="plain" aria-label={ariaLabel} onClick={onClick} className={className} {...props}>
{children}
Copy link
Collaborator

@mturley mturley Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Spacing here is still all messed up. I'll take a look at the config

@mturley
Copy link
Collaborator

mturley commented Aug 30, 2019

Yeah, weirdly enough we have "indent": [true, "spaces", 2], in the tslint.json, but tslint seems to be ignoring it.

yarn lint:ts | grep ChipButton
error Command failed with exit code 2.
ERROR: /Users/mturley/git/patternfly-react/packages/patternfly-4/react-core/src/components/ChipGroup/ChipButton.tsx:2:1 - Too many spaces after 'import'
ERROR: /Users/mturley/git/patternfly-react/packages/patternfly-4/react-core/src/components/ChipGroup/ChipButton.tsx:2:32 - Too many spaces before 'from'

@mturley mturley dismissed their stale review August 30, 2019 14:37

Some rules seem to be ignored

@mturley
Copy link
Collaborator

mturley commented Aug 30, 2019

Yikes. @redallen, this indent rule being broken is a bug that they just straight-up closed because TSLint is being deprecated. palantir/tslint#2814

Maybe that's reason enough to switch to tslint-plugin-prettier? at least until we switch to eslint

mturley
mturley approved these changes Aug 30, 2019
View reviewed changes
Copy link
Collaborator

@mturley mturley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, we'll fix these issues in the next one. Thanks @redallen!

@mturley
Copy link
Collaborator

mturley commented Aug 30, 2019

Merging this after discussion with @redallen and @dgutride 👍

@mturley mturley merged commit 023195c into patternfly:master Aug 30, 2019
@mturley mturley mentioned this pull request Aug 30, 2019
Merged
@mturley mturley mentioned this pull request Sep 27, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@mturley mturley mturley approved these changes

@dgutride dgutride dgutride approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dlabaj dlabaj

Labels

Infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Code style/formatting rules for TypeScript are not enforced by TSLint

6 participants

@redallen @patternfly-build @mturley @dgutride @dlabaj @tlabaj