amzn: enable amzn-nvme.c build without json-c dependencies checking #2627
Conversation
| #ifdef CONFIG_JSONC | ||
| if (!strcmp(cfg.output_format, "json")) | ||
| amzn_print_json_stats(&log); | ||
| else |
There was a problem hiding this comment.
Okay, looks reasonable. One thing we could do is to extend validate_output_format to inform the user that there is no json output supported and use this function here. But that is just bonus :)
There was a problem hiding this comment.
Thanks for adding validate_output_format. Instead parsing the cfg.output_format again you could use the flags variable directly
if (flag & JSON`)
amzn_print_json_stats
BTW, I would prefer to introduce an empty amzn_prin-json_stats implementation in case json-c is missing. This avoid sprinkling the ifdefs around in the code. IMO, this helps maintaining the code base in the long run.
There was a problem hiding this comment.
Yes I agree with you so just fixed this by the PR #2636. Thank you.
|
Oh I see that the build fails because suddenly we are building this plugin with the cross compilers... |
Only build json print codes with CONFIG_JSONC build option instead. Signed-off-by: Tokunori Ikegami <[email protected]>
To fix the cross compiler build fails. Signed-off-by: Tokunori Ikegami <[email protected]>
ikegami-t
force-pushed
the
amzn-json-dep
branch
from
5f085cc to
bd6850c
Compare
|
Fixed for the both comments. Thank you. |
|
Let's go with enabling these plugins first and we address the ifdefs afterwards. I'd rather go quickly here and have everything build with the cross tool chains! |
2 participants
Only build json print codes with CONFIG_JSONC build option instead.