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 6, 2021

Reviewing each commit one by one would be easier, because there are multiple commits, which are too messy when reviewing them together.

@Atry Atry marked this pull request as draft November 6, 2021 04:12
@Atry Atry force-pushed the split-baselinter branch 4 times, most recently from 5bec5e5 to 5f2c53f Compare November 6, 2021 05:35
@Atry Atry changed the title Separate the roles of BaseLinter into Linter and ProblemFinder Separate the roles of BaseLinter into Linter and LintRule Nov 6, 2021
@Atry Atry changed the title Separate the roles of BaseLinter into Linter and LintRule Separate BaseLinter into Linter and LintRule Nov 6, 2021
@Atry Atry marked this pull request as ready for review November 8, 2021 17:45
@Atry Atry requested a review from fredemmott November 8, 2021 17:48
@Atry Atry force-pushed the split-baselinter branch 4 times, most recently from c3577fb to e3d71d6 Compare November 8, 2021 22:20
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.

The implementation/structure seem great, but the naming is very confusing. Changing it will be a BC break, but I think the clarity/tech-debt avoidance is worth it here.

@Atry Atry force-pushed the split-baselinter branch from e3d71d6 to adc9919 Compare November 9, 2021 04:48
@Atry Atry marked this pull request as draft November 9, 2021 16:55
@Atry Atry force-pushed the split-baselinter branch from adc9919 to b4907a5 Compare November 9, 2021 17:06
@Atry Atry marked this pull request as ready for review November 9, 2021 17:07
@Atry Atry force-pushed the split-baselinter branch from b4907a5 to 91f6f43 Compare November 9, 2021 17:09
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.

Fine to merge as-is, either address before merging or a separate PR

@Atry Atry force-pushed the split-baselinter branch 2 times, most recently from 251f6dc to 31f3766 Compare November 9, 2021 19:58
@Atry Atry force-pushed the split-baselinter branch from 31f3766 to 717b695 Compare November 9, 2021 19:59
@Atry Atry changed the title Separate BaseLinter into Linter and LintRule Separate SingleRuleLinter into Linter and LintRule Nov 9, 2021
@Atry Atry merged commit 281e02c into hhvm:main Nov 9, 2021
@Atry Atry deleted the split-baselinter branch November 9, 2021 20:19
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