<div dir="ltr">On Tue, Jun 4, 2013 at 10:17 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
<br>
================<br>
Comment at: clang-tidy/ClangTidyModule.h:29<br>
@@ +28,3 @@<br>
+  virtual ~ClangTidyModule() {}<br>
+  virtual void addChecks(ClangTidyCheckVector *Checks) = 0;<br>
+};<br>
----------------<br>
</div><div class="im">Manuel Klimek wrote:<br>
> Daniel Jasper wrote:<br>
> > Manuel Klimek wrote:<br>
> > > I don't think the typedef buys us much here, but I might be alone with my opinion...<br>
> > As you have just asked my to change it above and that change would already have required me to change more than a handful of instances, I think you have proven yourself wrong ;-).<br>
> Well, I don't consider changing 10 lines of code a big issue; when I say I think it doesn't buy us much, I think that it doesn't aid readability :P<br>
</div>Wait until we have 100 such modules ;-)...<br>
<div class="im"><br>
================<br>
Comment at: clang-tidy/ClangTidyModule.h:21<br>
@@ +20,3 @@<br>
+<br>
+typedef std::pair<StringRef, ClangTidyCheck *> NamedCheck;<br>
+typedef SmallVector<NamedCheck, 16> ClangTidyCheckVector;<br>
----------------<br>
</div><div class="im">Manuel Klimek wrote:<br>
> Daniel Jasper wrote:<br>
> > Manuel Klimek wrote:<br>
> > > I'd probably use a getName() in ClangTidyCheck instead.<br>
> > The point is that I want this to be a configuration feature of the Module instead of an implementation detail of the check. I think we might arrive at a point where the same Check-class is used with slightly different options in several modules and thereby with different names.<br>

> Wouldn't that mean that it's even more strongly coupled to the Check class? Anyway, I don't feel too strongly about it.<br>
</div>I don't understand what you mean by that?<br>
<br>
Imagine, e.g. we would have a namespace comment check in both LLVM and Google and only the actually inserted comment would be slightly different. I would then put the NamespaceCommentCheck class into a common directory, make the inserted comment a parameter and let both the LLVMModule and the GoogleModule export it, e.g. as llvm-namespace-comments and google-namespace-comments.<br>

<br>
But I also don't feel strongly and think that we can only make more educated decisions once we have a few more checks implemented.<br></blockquote><div><br></div><div style>+1 to the last sentence :)</div><div style>
<br></div></div></div></div>