You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Breaking off from #47. There are two ideas at play here:
We have a lot of shortened names which are hard to read:
I would also like to suggest renaming of functions and variables. UpdtRdyLst_ is not much shorter than UpdateRdyList_ or UpdateReadyList_, but those are much easier to read. There's a pervasive removal of vowels: Dynmc, PrirtyLst, RcrsvScsr.
Some shortenings are reasonable (Reg), but there should be very few of these. The pattern I've noticed is that I'm generally fine if it's a single syllable of the larger word, rather than the larger word with vowels removed: Reg vs Rgstr
Many of our names break the clang-tidy check set up by LLVM:
To be clear on the state of this: Fix clang-tidy warnings #77 fixed the majority of these cases. What's left to fix is the casing of our variable names.
This is not an issue that can be fixed in one PR. It must be split into many. This is also significantly easier if we practice short branches rather than long-lived feature branches, else merge conflicts will be rampant and hard to fix.
Some advice on tackling this:
For expanding shortened names:
If the expansion can be accomplished by a script, it can be worth committing said script to the repo to aide in merge conflict resolution.
For fixing the remaining clang-tidy check:
The check needs to be re-enabled in some form. Currently, it's not very visible in the CI checks. Ideally, it would complain for new names which do not follow the rules.
The text was updated successfully, but these errors were encountered:
I think Visual Studio allows for automated renaming (F2 on a symbol) -- this may only be with C/C++ (Microsoft) extension. I haven't tried this feature, so I can't speak on its reliability, however it could be useful here.
As a side note, earlier I defended the weird vowel-removed naming scheme, however I have grown to dislike it -- I agree with renaming.
Actually, a better idea might be to have two versions of the function for some period of time (e.g. two months) to enable people to merge the changes into their branch. That is, keep the old function name around as an alias for the new name so that the migration can be done non-atomically.
For example:
// Before:voidUpdtRdyLst_(...) {
// The code
}
// After, pretending this change happened 2021-11-07:voidupdateReadyList_(...) {
// The code
}
[[deprecated("Renamed to updateReadyList_. Will be removed after 2022-01-07")]]
voidUpdtRdyLst_(...) {
updateReadyList_(...);
}
Breaking off from #47. There are two ideas at play here:
We have a lot of shortened names which are hard to read:
Many of our names break the clang-tidy check set up by LLVM:
This is not an issue that can be fixed in one PR. It must be split into many. This is also significantly easier if we practice short branches rather than long-lived feature branches, else merge conflicts will be rampant and hard to fix.
Some advice on tackling this:
The text was updated successfully, but these errors were encountered: