Skip to content
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

Fix AvoidSinglePipeOperator false negative in all inner expressions #697

Merged
merged 7 commits into from
Feb 19, 2024

Conversation

Mersho
Copy link
Contributor

@Mersho Mersho commented Feb 13, 2024

No description provided.

@knocte
Copy link
Collaborator

knocte commented Feb 13, 2024

@webwarrior-ws please review

@Mersho Mersho force-pushed the FalseNegativeAvoidSinglePipeOperator branch 3 times, most recently from 6f09eda to b83de98 Compare February 14, 2024 11:45
@knocte
Copy link
Collaborator

knocte commented Feb 15, 2024

@webwarrior-ws all good now?

@Mersho Mersho force-pushed the FalseNegativeAvoidSinglePipeOperator branch 4 times, most recently from f995950 to a392d8c Compare February 15, 2024 09:06
Copy link
Contributor

@webwarrior-ws webwarrior-ws left a comment

Choose a reason for hiding this comment

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

Overall the code ended up being not easy to understand, but at least it looks like it works. Though I'd add a test case with 3 pipe operators, just to make sure.

Switched to using `Seq.isEmpty` instead of comparing
array length.
@Mersho Mersho force-pushed the FalseNegativeAvoidSinglePipeOperator branch from a392d8c to ee54702 Compare February 15, 2024 09:38
This test was incorporated following the suggestion of
@webwarrior-ws to include a test case that utilizes three
pipe operators [1].

[1] fsprojects#697 (review)
@Mersho Mersho force-pushed the FalseNegativeAvoidSinglePipeOperator branch from ad62f26 to 8a93684 Compare February 19, 2024 10:04
@knocte
Copy link
Collaborator

knocte commented Feb 19, 2024

@Mersho please re-run CI build of the commit titled "AvoidSinglePipeOperator: fix for all inner exprs"

@Mersho
Copy link
Contributor Author

Mersho commented Feb 19, 2024

@Mersho please re-run CI build of the commit titled "AvoidSinglePipeOperator: fix for all inner exprs"

Done.

@knocte knocte merged commit 01c9bc9 into fsprojects:master Feb 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants