MyDeveloperDay added a comment.

I'm very much a "clang-format all the things" kind of person which is why I 
added C# and JSON support, as someone who moves between languages all day every 
day I want one tool to rule them all.. I'd love it if it would format YAML (but 
thats another story)..

For me this is a noble patch, but its just WAY too big, I'm really struggling 
to get through even half of it, and I've got my super xray specs on it because 
I think when I added C# support I did it in smaller phases and I didn't have 
full support out of the box, (and others came in and added it later), I feel 
the smaller patches can be reviewed more easily even if the full language 
support isn't quite there from day one.

This looks like a thorough implementation, but along the way you've changed 
things that possibly annoyed you, which I think are somewhat unrelated to 
adding verilog (although they made adding it easier), but its just making it 
patch so hard to get though thoroughly.   (and I'm not even the most critical 
of the reviewers), I think some of the other might eat you alive!! ;-)

but lets me say, my review comment might seem overly negative, I don't intend 
them that way, I think this could make clang-format highly usable to many new 
engineers, and this is a noble effort. I'm just not sure how comfortable every 
is going to be about dropping this.

It could have quite an impact on downstream forks.

Do we need to consider other potential styles of Verilog? 
https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md & 
https://cs233.github.io/verilogstyle.html are you building in the ability for 
Verilog specific items to be formatted in different way? (if that's even a 
thing)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121758/new/

https://reviews.llvm.org/D121758

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to