On 16/10/20 3:28 pm, Sebastian Huber wrote: > On 16/10/2020 01:36, Chris Johns wrote: >> On 16/10/20 4:23 am, Sebastian Huber wrote: >>> >>> rtems-fdt.c, rtems-fdt-shell.c and cpukit/include/rtems/rtems-fdt.h >>> seem to be dead code. They implement a shell command `fdt` but that >>> command is not part of the shell nor of any macro in >>> cpukit/include/rtems/shellconfig.h. >> >> This comment is wrong and should not have been in the commit message and >> should >> _not_ have been pushed. As I stated in my review it could be simple issue of >> not >> being in the config file and documentation. I feel EB needs to be more >> restrained in its view of what is used and not used in RTEMS and someone new >> the >> community might be advised to take a simple approach with wording when >> posting >> patches. > > The code in question > > * had warnings,
Yes these have appeared. > * is not integrated in the shell configuration, Yes this is what I pointed out is the issue in my review but then I saw it had been pushed. > * has no tests, And all the other commands have tests? > * no documentation, and Yes this is something that needs to be fixed. > * is in the architecture-independent area, Yes. FDT does not need to be limited to just device set up and bootloaders. It is used in others ways such as managing the address space of programmable logic without impacting on software. > so a comment that says this seems to be dead code is justified from my point > of > view. Yes and a comment or question asking is more than fine. A statement in a commit that is pushed while I am sleeping is not. If there is a situation like this please raise a ticket and it can serve a discussion point and a reminder. Commits messages not a place we review to see what needs to be sorted out. >> Please revert the change and then wait for the process to complete on the >> devel >> mailing list to complete. I will ack the patches when _I_ am OK with them. > > Yes, sorry, the review time was too short. Yes and thanks :) Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
