Skip to content

Conversation

@pjreiniger
Copy link
Contributor

Offline discussion makes it seem like @virtuald might not love this, but I figured I would at least make the PR. This turns the styleguide on for the non-python robotpy files. The overwhelming amount of changes were related to whitespace, followed by some IWYU for standard library headers.

Python files are excluded because I think import order matters for some of them, and some tests cases are currently just "make sure this can be imported".

I broke it down into a a couple of commits to make the changes more ovbious

  1. Update the styleguide.
  2. Run the styleguide, which fails because of 3 below. However this includes pretty much all of the whitespace changes
  3. Update header / template includes to have #pragma once. This allows wpiformat to get further along in the process
  4. Fix all of the warnings. Mostly IWYU, but also some c-style casts and single arg constructors should be explicit
  5. Rerun the formatter to clean up the whitespace from 4
  6. Fixup clang-tidy warnings. Mostly use braces for one line if statements and collapsing nested namespaces

@pjreiniger pjreiniger requested review from a team and PeterJohnson as code owners November 11, 2025 00:58
@github-actions github-actions bot added component: ntcore NetworkTables library component: wpiutil Utility library component: wpilibc WPILib C++ component: hal Hardware Abstraction Layer component: wpimath Math library component: wpinet Networking library component: apriltag AprilTag library 2027 2027 target labels Nov 11, 2025
@calcmogul
Copy link
Member

Some of the include order issues can be resolved with tweaks to the .styleguide file, but that's all getting replaced in #8350 anyway.

@pjreiniger
Copy link
Contributor Author

Some of the include order issues can be resolved with tweaks to the .styleguide file, but that's all getting replaced in #8350 anyway.

For python? Thats what I meant in reference to the import order mattering. I'm sure there is a way to disable the linter check anyway, but I figured it might be more acceptable to Dustin to just run it on these (I assume) not very frequently touched c++ files



#include "wpi/hal/HALBase.h"
#include <semiwrap_init.hal._wpiHal.hpp>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for these since they're locally generated files they should probably be "" includes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2027 2027 target component: apriltag AprilTag library component: hal Hardware Abstraction Layer component: ntcore NetworkTables library component: wpilibc WPILib C++ component: wpimath Math library component: wpinet Networking library component: wpiutil Utility library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants