Rewrite code_checker bash wrapper in python#81
Conversation
680a6b8 to
534b527
Compare
cf8658c to
57d6599
Compare
Szelethus
left a comment
There was a problem hiding this comment.
I just opened this file right next to src/codechecker_script.py in a terminal. There are some striking similarities:
- You declare a
log()and a_display_error()function, but there are already handy, and possibly more mature log functions in the other script:fail(),stage(),input_data(). _run_codechecker()competes withexecute(),prepare,analyze()- Both
_move_plist_files()and functionsrealpath(),resolve_plist_symlinks(),resolve_yaml_symlinks(),resolve_symlinks(),update_file_paths()are about results postprocessing, although they do it differently.
Some other questions arise when you consider the diffrences:
- Did we ever understand how do we relativize the paths in the plist files that come from the per-file rule? It seems to be only implemented in the monolithic rule.
- The per_file rule defines
_create_compile_commands_json_with_absolute_paths(), but I didn't immediately see anything similar for the other one. Why?
I am perfectly aware of the fact that I'm pointing out things which are specifically beyond the scope of your patch. Still, I'd at least like to see how these things would ideally look like, even if we don't solve them right away.
|
65ea5e1 to
bf07dfd
Compare
Szelethus
left a comment
There was a problem hiding this comment.
Okay, lets not get overambitious with this patch, I like the overall direction.
Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>
2d7c437 to
f988dd4
Compare
Szelethus
left a comment
There was a problem hiding this comment.
LGTM. There are miniscule things we could fuss about for weeks if we really wanted to, but the PR already provides more value and improvement than issues to fix. And honestly, those issues or cosmetic things are better fixed in a followup patch.
Why:
The inline bash script is very limiting.
What:
Rewrote the bash inline script into a separate Python file.
Adresses: