KEMBAR78
Don't ignore 5607 lint error under src/Migrations and fix an existing 5607 lint error by Atry · Pull Request #417 · hhvm/hhast · GitHub
Skip to content
This repository was archived by the owner on Dec 1, 2024. It is now read-only.

Conversation

@Atry
Copy link
Contributor

@Atry Atry commented Nov 30, 2021

This PR is to fix a 5607 linter error under src/Migrations. I reviewed $t->getLeading() !== $t->getLeadingWhitespace() and I believe the condition is always true, therefore, I removed the if clause.

@Atry Atry force-pushed the fix-5607-lint-error branch from aa9b408 to 2e90ff6 Compare November 30, 2021 22:14
@Atry Atry changed the title Fix 5607 lint error Don't ignore 5607 lint error under src/Migrations and fix an existing 5607 lint error Nov 30, 2021
@fredemmott
Copy link
Contributor

can you unstack this (and leave out the configuration change for now), so that it can be tested/merged/reverted independently?

@Atry
Copy link
Contributor Author

Atry commented Nov 30, 2021

Rebased

@Atry Atry force-pushed the fix-5607-lint-error branch from df9be4e to 01a62b3 Compare November 30, 2021 23:02
@Atry Atry requested a review from fredemmott December 2, 2021 18:10
Copy link
Contributor

@fredemmott fredemmott left a comment

Choose a reason for hiding this comment

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

Can you split out the configuration change please? The goal is to have independently bisectable/revertible commits for different issues.

@Atry Atry force-pushed the fix-5607-lint-error branch from 01a62b3 to 6851061 Compare December 2, 2021 18:58
"patterns": ["src/Migrations/*"],
"linterConfigs": {
"Facebook\\HHAST\\HHClientLinter": {
"ignore": [5624, 5639]
Copy link
Contributor

Choose a reason for hiding this comment

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

bump - this is what I'm referring to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the global ignore code list is [5607, 5624, 5639], so this commit actually enabled 5607 error code in comparison to the global ignore list

@Atry Atry merged commit a0edf13 into hhvm:main Dec 8, 2021
@Atry Atry deleted the fix-5607-lint-error branch December 8, 2021 18:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants