aaron.ballman added a comment. In D69813#1735804 <https://reviews.llvm.org/D69813#1735804>, @Szelethus wrote:
> In D69813#1734272 <https://reviews.llvm.org/D69813#1734272>, @Charusso wrote: > > > In D69813#1734193 <https://reviews.llvm.org/D69813#1734193>, @Szelethus > > wrote: > > > > > Hmm, so this checker is rather a collection of CERT rule checkers, right? > > > Shouldn't the checker name contain the actual rule name (STR31-C)? User > > > interfacewise, I would much > > > > > Implementationwise that sounds wonderful, these rules are fairly similar to > have a single checker responsible for them. The user interface however > (enabling different CERT rules) they should probably be split up into > separate checkers per rule, rather than options, wouldn't you agree? @NoQ? I'm not @NoQ, but I do agree that there should be a separate check per rule in terms of the UI presented to the user. The name should follow the rule ID like they do in clang-tidy, for some consistency there. > Also, `cert.str.Termination` sounds like a wonderful name, I don't insist on > the rule number being present in it, but at the very least, it should be in > the description. I think that the rule number should be in the name. I'd probably go with `cert.STR31-C` or `cert.str31-c` (so it's clear which CERT standard the rule came from). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69813/new/ https://reviews.llvm.org/D69813 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
