Copilot-Language
WIP: fix warnings
@yaitskov Thanks for the very extensive PR.
As a general rule, we want PRs to focus on one concern only (e.g., fixing warnings, fixing style, adding X feature, solving X bug, etc.). While a PR can sometimes address multiple things (solving a bug may require adding a feature, or solving a warning may require some style changes), we try to limit those to the cases where it is strictly necessary. In line with this, I've marked a few changes that I believe are not really addressing a warning, so they should be part of a different PR.
As you may have seen by now, it's going to take a while to review it all. Because of that, as other folks in the team start making changes towards Copilot 4.7, you may experience conflicts when trying to rebase your branch on top of their changes.
I think we'd be more successful getting your many changes adopted if we split this PR into multiple PRs, one per library, one per kind of change (e.g, warnings related to unused imports, warnings related to top-level functions without signatures, etc.), or even per library and per kind of change (especially in large libraries like copilot-theorem).
Please note that this is not a complaint. I'm appreciative of your PR, and I'm trying to figure out the best way to get changes adopted without taking too much time or effort on either side.
Also, a heads up: for any contributions to Copilot we need contributors to sign one of these two forms (whichever applies to their situation): https://github.com/Copilot-Language/copilot-discussion/tree/master/CLAs.
You'll only need to do this once.
Please send that, filling in all the details it asks for and signing it, to [email protected]. Thanks!
I think we'd be more successful getting your many changes adopted if we split this PR into multiple PRs, one per library,
OOPS
If formatting is that important - how about a PR with just automatic reformat and then do reformat in precommit hook or at least trigger by hand before openings a PR?
HLS, besides GHC, hints lot of minor issues (though I've seen fromJust among them which is partial) - it was hard to navigate with shortcuts to real warnings. They impose some sort of noise distracting attension.
We should agree to open a ticket to resolve all of them or specify which one should be disabled with a snippet of HLS config.
eg HLS hint doesn't like var names _ and encourage to append a suffix after _.