Skip to content

Conversation

@etoledano
Copy link
Contributor

Add and apply .clang-format to all *.cpp and *.h files.

This PR replaces this one, from a branch rather than a fork.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Apr 22, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@etoledano etoledano force-pushed the clang branch 3 times, most recently from e7af6a6 to 2feed42 Compare April 22, 2025 08:46
@etoledano
Copy link
Contributor Author

/ok to test 2feed42

@etoledano etoledano requested a review from aranadive April 22, 2025 08:49
@etoledano etoledano force-pushed the clang branch 2 times, most recently from 538ea02 to fc92ca0 Compare April 22, 2025 09:04
@etoledano etoledano marked this pull request as ready for review April 22, 2025 09:07
@etoledano
Copy link
Contributor Author

/ok to test fc92ca0

Add and apply .clang-format to all *.cpp and *.h files.

Signed-off-by: Eylon Toledano <[email protected]>
@etoledano
Copy link
Contributor Author

/ok to test bcdac8b

@tstamler
Copy link
Contributor

@etoledano I think my plan for merging this is going to be right after 0.2.0. The merge conflicts are going to be quite difficult, but once we get everything merged for 0.2.0 on Friday there should be a nice downtime where we aren't merging anything new where we can apply all these changes.

@aranadive suggested we also shrink this down to a smaller PR to start with, almost like a proof of concept we can manually review to ensure everyone approves of the style, then expand back to cover everything.

@etoledano
Copy link
Contributor Author

@etoledano I think my plan for merging this is going to be right after 0.2.0. The merge conflicts are going to be quite difficult, but once we get everything merged for 0.2.0 on Friday there should be a nice downtime where we aren't merging anything new where we can apply all these changes.

@aranadive suggested we also shrink this down to a smaller PR to start with, almost like a proof of concept we can manually review to ensure everyone approves of the style, then expand back to cover everything.

I am not in any rush, just sent a mail to initiate a discussion to hopefully push it by mid May.
Regarding POC commit, basically anyone can copy-paste this .clang-format, change it and apply it on the code to visualize.

@aranadive
Copy link
Contributor

@etoledano I think my plan for merging this is going to be right after 0.2.0. The merge conflicts are going to be quite difficult, but once we get everything merged for 0.2.0 on Friday there should be a nice downtime where we aren't merging anything new where we can apply all these changes.
@aranadive suggested we also shrink this down to a smaller PR to start with, almost like a proof of concept we can manually review to ensure everyone approves of the style, then expand back to cover everything.

I am not in any rush, just sent a mail to initiate a discussion to hopefully push it by mid May. Regarding POC commit, basically anyone can copy-paste this .clang-format, change it and apply it on the code to visualize.

Lets add the clang format file in a separate commit and update README with clang format instructions. That way, anyone can take the file and apply it to see what works. If someone wants to update clang file based on output, we should do that before applying to all source files.

@etoledano
Copy link
Contributor Author

etoledano commented May 8, 2025

@etoledano I think my plan for merging this is going to be right after 0.2.0. The merge conflicts are going to be quite difficult, but once we get everything merged for 0.2.0 on Friday there should be a nice downtime where we aren't merging anything new where we can apply all these changes.
@aranadive suggested we also shrink this down to a smaller PR to start with, almost like a proof of concept we can manually review to ensure everyone approves of the style, then expand back to cover everything.

I am not in any rush, just sent a mail to initiate a discussion to hopefully push it by mid May. Regarding POC commit, basically anyone can copy-paste this .clang-format, change it and apply it on the code to visualize.

Lets add the clang format file in a separate commit and update README with clang format instructions. That way, anyone can take the file and apply it to see what works. If someone wants to update clang file based on output, we should do that before applying to all source files.

Done in a new PR (from a fork) but it should fail since once the clang-format is updated its already applied, so it expects it to pass.

The contribution guidelines already covers .clang-format

- The code style convention is enforced by common formatting tools
  for a given language (such as clang-format for c++, black for python).

@etoledano etoledano closed this May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants