Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove superfluous Tofino include. Make Tofino include paths fully qualified. #4998

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Nov 4, 2024

No description provided.

Signed-off-by: fruffy <fruffy@nyu.edu>
@fruffy fruffy added the tofino Topics related to the Tofino switch and back end. label Nov 4, 2024
@fruffy fruffy requested a review from hanw November 4, 2024 02:02
@fruffy fruffy marked this pull request as ready for review November 4, 2024 02:02
@@ -16,7 +16,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include "bf-p4c/arch/add_t2na_meta.h"
#include "backends/tofino/bf-p4c/arch/add_t2na_meta.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be just #include "add_t2na_meta.h" as it is in the same directory. No need for a full path.

Lots of other places this could be cleaned up too -- possibly a separate cleanup change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess this is a stylistic choice. Not many guides mention it but Google style guide recommends to qualify all paths: https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

I guess to avoid accidental conflicts with includes?

I do not have a strong preference but the Tofino back end is not the only part which doesn't follow this style. So this likely should be a separate cleanup change.

@hanw
Copy link
Contributor

hanw commented Nov 4, 2024

Thanks for cleaning this up.

@fruffy fruffy added this pull request to the merge queue Nov 4, 2024
Merged via the queue into main with commit c16800c Nov 4, 2024
21 checks passed
@fruffy fruffy deleted the fruffy/tofino_includes branch November 4, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tofino Topics related to the Tofino switch and back end.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants