Skip to content

Conversation

@w1ldptr
Copy link
Contributor

@w1ldptr w1ldptr commented Jul 15, 2025

What?

Create a config file for clang-tidy tool in order to enforce naming style of types, functions, and variables according to NIXL code style.

Why?

Both internal and external contributors have often been naming types, functions, and variables incorrectly. Moreover, due to complex NIXL naming conventions these mistakes often slip past code review.

How?

Clang-tidy is a tool readily available in Ubuntu and other distro's repositories similarly to clang-format that is already used by NIXL. Excerpt from the tool output with provided config on a NIXL source:

../../home/vladbu/src/nixl/src/plugins/ucx/ucx_backend.cpp:236:59: warning: invalid case style for parameter 'devId' [readability-identifier-naming]
  236 | int nixlUcxEngine::vramUpdateCtx(void *address, uint64_t  devId, bool &restart_reqd)
      |                                                           ^~~~~
      |                                                           dev_id
  237 | {
  238 |     int ret;
  239 |     bool was_updated;
  240 |
  241 |     restart_reqd = false;
  242 |
  243 |     if(!cuda_addr_wa) {
  244 |         // Nothing to do
  245 |         return 0;
  246 |     }
  247 |
  248 |     ret = cudaCtx->cudaUpdateCtxPtr(address, devId, was_updated);
      |                                              ~~~~~
      |                                              dev_id
../../home/vladbu/src/nixl/src/plugins/ucx/ucx_backend.cpp:288:14: warning: invalid case style for method 'is_complete' [readability-identifier-naming]
  288 |         bool is_complete() const { return _completed; }
      |              ^~~~~~~~~~~
      |              isComplete
../../home/vladbu/src/nixl/src/plugins/ucx/ucx_backend.cpp:322:12: warning: invalid case style for struct 'Notif' [readability-identifier-naming]
  322 |     struct Notif {
      |            ^~~~~
      |            notif
  323 |             std::string agent;
  324 |             nixl_blob_t payload;
  325 |             Notif(const std::string& remote_agent, const nixl_blob_t& msg)
      |             ~~~~~
      |             notif
  326 |                     : agent(remote_agent), payload(msg) {}
  327 |     };
  328 |     std::optional<Notif> notif;
      |                   ~~~~~
      |                   notif

@github-actions
Copy link

👋 Hi w1ldptr! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

Co-authored-by: Raul Akhmetshin <[email protected]>
Signed-off-by: Vlad Buslov <[email protected]>
@w1ldptr
Copy link
Contributor Author

w1ldptr commented Jul 16, 2025

/build

@w1ldptr
Copy link
Contributor Author

w1ldptr commented Jul 16, 2025

/build

@w1ldptr
Copy link
Contributor Author

w1ldptr commented Jul 16, 2025

/ok to test d8254e0

@yosefe yosefe merged commit 0f56527 into ai-dynamo:main Jul 17, 2025
10 checks passed
@w1ldptr w1ldptr deleted the clang-tidy branch July 21, 2025 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants