)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"2e19c52f5c6210fb60e77b7d28bb60394ef20c18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a2cb9d23_15bd1dfa","updated":"2024-10-28 14:31:58.000000000","message":"I have in my personal TODO list the task to reorganize the include files.\n- We cannot reorder them because of some cyclic dependency\n- not all the dependencies are present\n- some dependency can be dropped by adding `struct xxx` in the include and dropping including the file that fully specifies the `struct`.\n\nAll this work should also improve the incremental build time, after an include is modified by a cherry-pick or code editing.","commit_id":"36557cd1ebf0312b3da0e98a3551148b859b11b6"},{"author":{"_account_id":1001667,"name":"Jan Matyas","email":"jan.matyas@codasip.com","username":"JanMatCodasip"},"change_message_id":"38a5c56ad11603b79f05e39924aa9a8b22ab4e56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a60a4b21_5549f958","in_reply_to":"a2cb9d23_15bd1dfa","updated":"2024-10-29 06:12:18.000000000","message":"Hi Antonio, \n\nthank you for your reply.\n\nWhen I was experimenting with the includes and thinking about the current state, my preferred course of action would be:\n\n1) Unify the include style. In the current code, includes with quotes and angle brackets are used inconsistently. Preferred: Angle brackets for system includes, quotes for OpenOCD\u0027s own header files.\n\n2) Unify paths to the includes. Currently the paths are inconsistent too: Some includes are relative to the current file, some includes have relative paths to the src/ directory. Preferred: Use relative path to src/ in all cases.\n\n3) Introduce an automated tool to ensure the rule \"include what you use\". Possible tools are for example:\n- include-clearner from clang-tidy [1]\n- include-what-you-use [2]\n\nThe first of them seems to have issues that prevent it from being used on OpenOCD codebase [3][4]. The second tool I haven\u0027t yet tried.\n\n4) Introduce automated tool to sort the includes in a unified way. E.g. clang-format can help here [5].\n\nDo you have a similar vision or would you prefer to handle the includes differently?\n\nRegards, Jan\n\n\n[1] https://clang.llvm.org/extra/clang-tidy/checks/misc/include-cleaner.html\n[2] https://github.com/include-what-you-use/include-what-you-use\n[3] https://github.com/llvm/llvm-project/issues/113927\n[4] https://github.com/llvm/llvm-project/issues/113926\n[5] https://clang.llvm.org/docs/ClangFormatStyleOptions.html#sortincludes","commit_id":"36557cd1ebf0312b3da0e98a3551148b859b11b6"},{"author":{"_account_id":1000021,"name":"Antonio Borneo","email":"borneo.antonio@gmail.com","username":"borneoa"},"change_message_id":"6cc05a9fdcd34d522e142600b3a2a81066557f4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a936bd46_67133512","in_reply_to":"a60a4b21_5549f958","updated":"2024-10-29 09:18:06.000000000","message":"Hi Jan,\n\n\u003e 1) Unify the include style. In the current code, includes with quotes and angle brackets are used inconsistently. Preferred: Angle brackets for system includes, quotes for OpenOCD\u0027s own header files.\n\nAgree!\n\n\u003e 2) Unify paths to the includes. Currently the paths are inconsistent too: Some includes are relative to the current file, some includes have relative paths to the `src/` directory. Preferred: Use relative path to `src/` in all cases.\n\nThe build of OpenOCD is by subsystems, so we where using path relative to `src/` for \u0027external\u0027 files, path relative to current file for includes belonging in the same subsystem.\nI agree this is confusing and we can switch to have everything relative to `src/`.\nWe should also modify the Makefiles to drop ` -I. ` in order to enforce using only the path relative to `src/`.\n\n\u003e 3) Introduce an automated tool to ensure the rule \"include what you use\".\n\nThanks for the hint. I was doing some \u0027manual\u0027 trial to detect the dependencies. Having some tool should be way better.\nI never used any of the tools you propose, I cannot provide any feedback so far.\nFeel free to use them offline, propose patches to correct the dependencies and, at the end, propose a way to automate it in gerrit and/or local build.\n\n\u003e 4) Introduce automated tool to sort the includes in a unified way. E.g. clang-format can help here [5].\n\nChecking the order is a minor overhead for the maintainers, but yes, why not.\n\n\u003e Do you have a similar vision or would you prefer to handle the includes differently?\n\nNo, not differently but with more constraints.\nLet\u0027s take the example of `target/target.h`. Today it is included everywhere because `struct target` is used everywhere in OpenOCD.\nBUT 99% of the code outside folder `target/` only use `struct target *` and never need to know the fields inside the struct.\nWe should completely break `target/target.h` in two files:\n- moving most of its content in `target/target_internals.h` that is ONLY used by files in `target/`,\n- keeping in `target/target.h` ONLY what is exported and used by files outside `target/`. It should declare `struct target;` as an opaque object with hidden internal structure.\n\nFor the 1% of files outside `target/` that still require internals of the struct, we should think about adding new API in `target/target.h` to drop this link.\n\nOf course, what I write for `target/target.h` should be extended to the other subsystems.","commit_id":"36557cd1ebf0312b3da0e98a3551148b859b11b6"}]}