On Mon, Jun 22, 2015 at 6:15 PM, David Blaikie <[email protected]> wrote:
> > > On Mon, Jun 22, 2015 at 2:47 AM, Alexander Kornienko <[email protected]> > wrote: > >> Author: alexfh >> Date: Mon Jun 22 04:47:44 2015 >> New Revision: 240270 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=240270&view=rev >> Log: >> Fixed/added namespace ending comments using clang-tidy. NFC >> > > Is this actually an LLVM or Clang coding convention? It doesn't seem to be > & I'm not sure it's one worth adopting... might be worth some discussion > before we go in this direction? (perhaps there has been discussion in a > review thread, etc & I've missed it - helpful to call that out in the > commit message (& probably put this in the LLVM coding conventions if we > are going in that direction)) > I was having an impression that there's a convention to use namespace ending comments of this form, that's why I considered a purely mechanical change that doesn't need a discussion. However, now reading the coding standards <http://llvm.org/docs/CodingStandards.html#namespace-indentation>, I see that the relevant rule is much weaker, it just allows adding comments, not even recommends them: If it helps readability, feel free to add a comment indicating what namespace is being closed by a }. and also the style of the comment used in the example is slightly different: } // end namespace llvm instead of } // namespace llvm used in this patch. So an agreement on the style (and necessity) of the namespace ending comments prior to committing the patch wouldn't hurt. Would you like me to revert the patches before we figure out what the desired state is? Also, what the preferred solution here would be? 1. leave the comments (or the lack thereof) alone, don't even fix incorrect or inconsistent ones 2. only fix incorrect comments 3. fix incorrect comments and make all existing namespace ending comments consistent 4. 3. + add namespace ending comments to all namespaces spanning more than X lines 5. something else? In cases 2-4 we need to decide which format of namespace ending comments to use: a. // llvm b. // namespace llvm c. // end namespace llvm d. // end of namespace llvm e. // end llvm namespace f. something else This patch corresponds to 4b.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
