fsprojects / FSharpLint

Lint tool for F#
https://fsprojects.github.io/FSharpLint/
MIT License
300 stars 73 forks source link

A new rule that finds used underscore prefixed elements #643

Closed webwarrior-ws closed 6 months ago

webwarrior-ws commented 6 months ago

Supersedes #591

Fixes https://github.com/fsprojects/FSharpLint/issues/573

knocte commented 6 months ago

This branch has conflicts that must be resolved

@webwarrior-ws bad rebase ^ ; also, please push 1 by 1

knocte commented 6 months ago

@webwarrior-ws I think what you did wrong here when squashing is overlooking/disregarding this common pitfall about git: https://github.com/nblockchain/conventions/blob/master/docs/WorkflowGuidelines.md?plain=1#L245

webwarrior-ws commented 6 months ago

When I rebased there were no conflicts. Then I was squashing commits and something must have gone wrong there.

knocte commented 6 months ago

and something must have gone wrong there.

Exactly what I meant when I pointed to our documentation, please read this line: https://github.com/nblockchain/conventions/blob/master/docs/WorkflowGuidelines.md?plain=1#L245

knocte commented 6 months ago

@webwarrior-ws shouldn't 06fb857b44d9fa988e377ae07b39641ec312101e's CI be green instead of red?

webwarrior-ws commented 6 months ago

@webwarrior-ws shouldn't 06fb857's CI be green instead of red?

In this commit I made tests pass, but self-check fails revealing another bug. Then I added test for this case in next commit and fix in the last commit.

knocte commented 6 months ago

I think the first 7 commits need to be squashed, we don't gain anything by having them split in many, and their CI statuses are all red anyway (without having a failing test).

webwarrior-ws commented 6 months ago

Squashed first 7 commits

knocte commented 6 months ago

@webwarrior-ws commits c0559b8603750039d156fffaa7a4504a4749df5d and 886f06474801034d2a925ca375dab1dfe5caa81f can be squashed properly too. Also don't forget to address last commit (make test pass -> CI green). Thanks

knocte commented 6 months ago

@webwarrior-ws please note: as the quickfix commits (the last three) require more discussion, I've removed them from this PR. So, once I merge it, you can create new PR for those commits (e.g. cherry-picking them from this backup branch where I pushed them: https://github.com/knocte/FSharpLint/commits/addQuickFixToRule82/ ).