Skip to content

Conversation

@Szelethus
Copy link
Contributor

Why:
In anticipation of #98, where we remove the compilation database generation from per_file analysis and reuse compile_commands.bzl, we identified that the best point of entry might not be compile_commands_impl(), because we still need to check and parse the return values. Since we need the exact same ~20 lines of code that codechecker.bzl already uses, lets just turn that into a convenience function.

What:
Turn the handling/parsing of compile_commands_impl into a function.

Addresses:
Accessory to #98.

@Szelethus Szelethus requested a review from furtib January 15, 2026 08:32
@Szelethus Szelethus added the non-functional change ☮️ The patch doesn't change any functionality, e.g. refactoring, documentation, test-only. label Jan 15, 2026
Copy link
Contributor

@furtib furtib left a comment

Choose a reason for hiding this comment

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

LGTM

@Szelethus Szelethus force-pushed the refactor_comp_comm_impl_usage branch from 74b2e55 to 4c0e4cc Compare January 15, 2026 12:15
Copy link
Collaborator

@nettle nettle left a comment

Choose a reason for hiding this comment

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

To be honest I'm kind of against this change.
May we discuss this before submitting?

@Szelethus
Copy link
Contributor Author

Yes, sure, can you explain what about this change you think is troublesome? Is this a concern about an external factor, or is the patch incorrect?

@nettle
Copy link
Collaborator

nettle commented Jan 16, 2026

Hi @Szelethus,
What I meant is #145 - very simple function which can be used in #98 without any changes in src/codechecker.bzl
This is still very optional

@Szelethus Szelethus changed the title Greate a convenience function for interacting with compile_commands_impl Create a convenience function for interacting with compile_commands_impl Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

non-functional change ☮️ The patch doesn't change any functionality, e.g. refactoring, documentation, test-only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants