drt: factor shared internal headers into private hdrs target#10427
drt: factor shared internal headers into private hdrs target#10427maliberty wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the src/drt/BUILD file by introducing a new cc_library target, drt_private_hdrs, to consolidate private headers used by both the drt and ui targets. It also updates the visibility of the db_hdrs target and adjusts dependencies across the module. The reviewer suggests adding the "include" directory to the includes attribute of the new target to ensure proper header resolution and expanding its visibility to //src/drt:__subpackages__ to allow access for unit tests.
|
clang-tidy review says "All clean, LGTM! 👍" |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7c99992e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Move the 15 headers that were duplicated across :drt and :ui srcs in src/drt/BUILD into a new :drt_private_hdrs cc_library with //visibility:private, so they're owned in one place and unreachable from outside //src/drt. Restrict :db_hdrs visibility to //src/drt:__subpackages__ so it stays accessible to drt unit tests without leaking outside the module. Move FlexMazeTypes.h from :db_hdrs to :base_types, where the foundational types already live. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
openroad-ci
force-pushed
the
drt-private-hdrs
branch
from
1a7c999 to
b39f811
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| "src/ta/FlexTA.h", | ||
| ], | ||
| includes = ["src"], | ||
| visibility = ["//visibility:private"], |
There was a problem hiding this comment.
This is the default so you typically don't specify it
2 participants
Summary
:drtand:uisrcs insrc/drt/BUILD(AbstractGraphicsFactory.h,distributed/drUpdate.h,dr/AbstractDRGraphics.h,dr/FlexDR.h,dr/FlexGridGraph.h,dr/FlexWavefront.h,frDesign.h,frRegionQuery.h,gc/FlexGC.h,global.h,pa/AbstractPAGraphics.h,pa/FlexPA.h,pa/FlexPA_unique.h,ta/AbstractTAGraphics.h,ta/FlexTA.h) into a new:drt_private_hdrscc_librarywith//visibility:private.:db_hdrsvisibility to//src/drt:__subpackages__so drt unit tests can still depend on it (the only external user today is//src/drt/test) but nothing outside//src/drtcan.FlexMazeTypes.hfrom:db_hdrsto:base_types, where the foundational types it sits next to already live.Type of Change
Impact
Build-system only; no behavior change. Header duplication across two targets in the same package is eliminated while preserving
layering_checksemantics, and the data-model surface is no longer visible to packages outside//src/drt.Verification
bazel build //src/drt:drt //src/drt:ui //src/drt:db_hdrs //src/drt:base_types //src/drt/test/...).Related Issues
None.