stevedlawrence commented on a change in pull request #545:
URL: https://github.com/apache/daffodil/pull/545#discussion_r624161157
##########
File path:
daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libcli/daffodil_main.c
##########
@@ -15,13 +15,13 @@
* limitations under the License.
*/
-#include <stdio.h> // for NULL, perror, FILE, fclose, fflush, fopen,
stdin, stdout
-#include <string.h> // for strcmp
-#include "daffodil_argp.h" // for daffodil_parse, daffodil_parse_cli,
daffodil_unparse, daffodil_unparse_cli, daffodil_cli, parse_daffodil_cli,
DAFFODIL_PARSE, DAFFODIL_UNPARSE
-#include "errors.h" // for continue_or_exit, Error, print_diagnostics,
PState, UState, ERR_FILE_CLOSE, ERR_FILE_FLUSH, ERR_FILE_OPEN,
ERR_INFOSET_READ, ERR_INFOSET_WRITE
-#include "infoset.h" // for walkInfoset, InfosetBase, rootElement, ERD,
VisitEventHandler
-#include "xml_reader.h" // for xmlReaderMethods, XMLReader
-#include "xml_writer.h" // for xmlWriterMethods, XMLWriter
+#include <stdio.h> // for NULL, FILE, perror, fclose, fopen, stdin,
stdout
+#include <string.h> // for strcmp
+#include "daffodil_getopt.h" // for daffodil_cli, parse_daffodil_cli,
daffodil_parse, daffodil_parse_cli, daffodil_unparse, daffodil_unparse_cli,
DAFFODIL_PARSE, DAFFODIL_UNPARSE
Review comment:
I don't have any problem with these comments, that said,
> If a developer is curious to learn more about a symbol, he or she can find
exactly where to look by searching the comments to find the right include
header and then open that .h file (also the .c file if both are in the
repository).
Most IDE's and even text editors used for coding have an easy way to jump
directly to the definition of a symbol. ctags+vim is my go-to, but there's lots
of options for a more efficient alternative than searching through #include
comments.
Your workflow to update these comments also seems pretty laborious. If it's
not automated, I would expect that once other developers get involved that
aren't used to this workflow it's going to be hard to keep these comments
updated and useful. The removal of superfluous #includes does seem like a
useful feature of iwyu though (I wasn't aware of this tool). Seems like there
might be some benefit it automating some of these capabilities.
FYI, I've spoken to some devs recently that sound interested in this
runtime2 and have similar FPGA goals, hoping to get them involved soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
