Merge distributed rule metadata files - #260
Conversation
We absolutely do not need the individual metadata files in the data folder, the merged one is enough
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
Please note that the structure of metadata.json, specifically version 2, is specified here:
https://github.com/Ericsson/codechecker/blob/master/docs/report_directory.md#metadata-structure
It might be wise to drop this somewhere in the new metadata_merge.py file.
| # Fail if the plist file version is different | ||
| assert json1["version"] == json2["version"] |
There was a problem hiding this comment.
I'll one up you: Fail if the version isn't 2.
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
It'd be nice to follow the official documents order of items.
Also, we could have a very nice unit test on this. And currently we have none :)
| json1_root = json1["tools"][0] | ||
| json2_root = json2["tools"][0] |
There was a problem hiding this comment.
*_root sounds like a poor name. How about *_first_tool or something like that?
| # Merge time; we assume here both json files describe jobs in | ||
| # the same analysis invocation, implying that the analysis start | ||
| # time is the lowest timestamp, and the end is the highest. | ||
| # Note: caching will break this assumption |
There was a problem hiding this comment.
That sounds like a bigger deal than this Note implies.
| assert json1_root["name"] == json2_root["name"] | ||
| # same CodeChecker version | ||
| assert json1_root["version"] == json2_root["version"] | ||
| # We assume that the list of enabled checkers haven't changed between runs. |
There was a problem hiding this comment.
What if we have several, differently configured codechecker_test targets, on which a main test target depends? Would each of those targets do their metadata merge individually?
Can we (should we) assert on this?
2 participants
Why:
We don't need multiple metadata files, just one.
What:
Addresses:
Fixes: #45
Depends on:
#79