feat(metadata): add cc flag parser#112
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new cc package for parsing C/C++ build metadata flags into a structured format, including support for sysroot extraction and linker flag identification. Feedback identifies a bug where linker flags followed by a space (e.g., -L /path) result in the argument being miscategorized as a compiler flag. Additionally, the CFLAGS field in the Metadata struct is currently unused and should either be populated or removed to avoid confusion.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
==========================================
+ Coverage 79.69% 80.48% +0.78%
==========================================
Files 35 36 +1
Lines 1921 2034 +113
==========================================
+ Hits 1531 1637 +106
- Misses 285 290 +5
- Partials 105 107 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
67e0cdb to
cdf8f33
Compare
cdf8f33 to
eb49f04
Compare
|
/review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the cc package to parse C/C++ build metadata flags into a structured Metadata object, including logic to categorize flags for compilation, linking, and sysroot. Feedback focuses on improving adherence to Go naming conventions for the Metadata struct and unifying field visibility. Several opportunities for code cleanup were also identified, such as removing unused constants, eliminating redundant string splitting for linker flags, and simplifying the sysroot assignment logic.
|
/review |
| // CFLAGS contains flags that apply specifically to C compilation. | ||
| CFLAGS []string |
There was a problem hiding this comment.
The comments for CFLAGS and CXXFLAGS say "flags that apply specifically to C/C++ compilation," but in practice only -std flags (with C or C++ standard values) are routed here — no other C/C++-specific flags get classified separately. This could mislead consumers into expecting e.g. -Wstrict-prototypes to land in CFLAGS. Consider narrowing the wording, e.g.:
// CFLAGS contains -std flags with C language standard values (e.g. c99, gnu11).
2044b50 to
bfd8a58
Compare
No description provided.