Open farmerpiki opened 2 months ago
Thanks! There are two things here, a note and a question for each:
Format: This repo uses a lot of hand-formatting here, including for .cpp2
code. I think the only answer to this part is to not just reformat an existing repo's code? What do other repos do, when different contributors may have different clang-format rules?
Tidy: If there are fixes needed for actual/potential bugs with mostly true positives, that would be useful information. What do other repos do, when a contributor may use a linter or static analysis tool that has imperfect rules that generate false positives?
so first of all most of these are set in advance for a repo, retroactively introducing them is not easy most of the time, however...
format: I'll do best effort to format with minimal changes if at all possible, for parts that have massive changes we could put them between // clang-format off and // clang-format on
tidy: this is a bit easier and could actually catch potential bugs as mentioned, for false positives once manually confirmed not to be a problem we can just annotate that line with NOLINT for that specific false positive, again we can start with a configuration that works with minimal changes and disable all the checks that would cause a lot of changes, gradually later we could reintroduce some of the checks that might be beneficial in catching bugs and other stuff
most of the repos which use this also have set up continuous integration and it's set up in github to automatically check pull requests and non-conforming ones are rejected and impossible to merge... there's also the possibility to automatically run these tools on each commit but I think that would just introduce other dependencies on contributors and get in the way of commiting quick-fixes...
I don't expect this to be easy, as for cpp2 we don't even have a linter or formatter yet but if we decide on a few rules for cpp1 we could at least make sure the generated code conforms to what standards we hold ourselves to...
For an idea of what I did so far... have a look here https://github.com/hsutter/cppfront/compare/main...farmerpiki:cppfront:main
if you prefer a solution let me know which way to go to minimize friction, I'm willing to accomodate here @hsutter that's why I asked the question in the first place at the moment my preference for the clang-tidy file I enable a category at a time, disable anything that creates a lot of noise, and the ones which require very minimal changes I leave in and do the changes as well I could do separate commits to fix these if you prefer so there are a lot more categories I plan to include, next one is modernize, but there I will definitely disable trailing return types for example
at least this way by explicitly disabling some lints we can see which one we care about more easily and enable them later in future commits perhaps
I think except for maybe adding a bit more work in case we roll back a commit there should be no issues for other developers. We shouldn't force linters on those that don't want to use them. For me personally I prefer having one enabled just to catch if I somehow forget my own rules and do something that might be potentially wrong. I will not worry about backwards compatibility for this stuff, it would just make me, and possibly others with similar setups less likely to make silly bugs.
(UNRELATED) I'd also like to do some performance work after: first idea is to memory map the file on the platforms that support it and hold it all as a string for those that don't ... and use std::vector
I also added modernize to it, with minimal changes @hsutter let me know if this is acceptable or I should go for more or less changes
added performance lints and made a pull request
https://github.com/hsutter/cppfront/pull/1253
if this gets accepted I will update it in the future, as it integrates (better) with my way of working
and rebased on top of main... although it was mergeable already
I'm trying to contribute but ran into a few issues.
By default my editor is configured to run clang-format on save and clang-tidy to show me possible fixes to different issues I might encounter. This generates a huge amount of noise while trying to commit and a lot of false positives due to functions defined in headers and stuff like that.
I can definitely turn these features off for this project, however I think others might run into the same issues and be discouraged from contributing due to these.
I'd be willing to write these default files if there are no objections to that. Just want to see some feedback first.