Attention is currently required from: Raul Rangel, Piotr Król. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59169 )
Change subject: utils: Add initial version of "remove_unused_code" script ......................................................................
Patch Set 2:
(22 comments)
File util/scripts/rm_unused_code:
https://review.coreboot.org/c/coreboot/+/59169/comment/8cf03a6a_ef84c9f5 PS1, Line 35: show_version() { : echo : _echo_color "${BOLD}${GREEN}" "${PROGNAME} version ${VERSION}" 1 : echo : }
nit: I would move this under _echo_color
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/d887b5e5_3d048f50 PS1, Line 41: _echo_color
Can you add a comment describing the args.
Added local variables with named arguments.
https://review.coreboot.org/c/coreboot/+/59169/comment/720d4442_dfa3a640 PS1, Line 50: test -n "${VERBOSE}"
You have `-e set above. […]
Removed _echo_debug, it's not currently used.
https://review.coreboot.org/c/coreboot/+/59169/comment/5eaa0d01_6ccd068e PS1, Line 72: BLD_DIR
Do you need to `shift` after pulling the arg off?
I added the shift at the end of the loop so if there's only the argument flag, no shift is needed. All of the other shifts have been removed.
https://review.coreboot.org/c/coreboot/+/59169/comment/e3a6e6b4_8675af60 PS1, Line 115: ${dir}
quote
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/a7e4939c_4ec5c960 PS1, Line 125: ${dir}
""
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/19c9b89e_ed1da576 PS1, Line 189: {
You could replace the {} these with () to make the function invoke the subshell.
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/775313ef_3004a1b3 PS1, Line 196: # find "${BLD_DIR}" -type f -exec touch -a --date="2020-01-01 00:00:00" {} ;
?
This was the old version of the code before I changed to git ls-files. Removing.
https://review.coreboot.org/c/coreboot/+/59169/comment/6e433fc0_b312dc2c PS1, Line 205: KEEP_FILES
Can you declare KEEP_FILES as an array above,then use "${KEEP_FILES[@]}"
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/32088282_9fea7646 PS1, Line 206: find
Why do we need the find? Can this be replaced with `touch "${BLD_DIR}/${file}"`?
The function works on files inside directories as well as just files. The find allows us to specify a directory and mark all the files inside. If it's an actual single file, the find is transparent.
https://review.coreboot.org/c/coreboot/+/59169/comment/d6a9c856_736fe376 PS1, Line 211: $1
""
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/863fcbf1_9997f347 PS1, Line 214: ${CONFIG_FILE}
""
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/d96b66e2_f5bd6cd5 PS1, Line 221: || exit 1
Your `-e` should take care this, but it doesn't hurt.
Right. This is left over from before I had the -e. Removed.
https://review.coreboot.org/c/coreboot/+/59169/comment/5e4b6898_7d420438 PS1, Line 228: MODIFIED_FILES
Could use `readarray MODIFIED_FILES < <(find ... […]
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/5f3df398_87029775 PS1, Line 228:
I think you can add a `-path ./.git -prune` to the find command and avoid the grep.
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/d3a0c76f_23957c0e PS1, Line 229: (echo "${MODIFIED_FILES}" | wc -l)
If you use an array you can use "${#MODIFIED_FILES[@]}"
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/3e9b9118_18e2b8cf PS1, Line 236: -exec rm {} ;
-delete
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/02cc9ccf_70e1f419 PS1, Line 249: CLEAN_DIR_LIST
I would make it an array: "${CLEAN_DIR_LIST[@]}"
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/d043db29_fd3e9474 PS1, Line 252: -exec rm {} ;
-delete
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/f85f2a05_8ad80faa PS1, Line 264: &&
Shouldn't need the && with `-e`
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/c011cb67_85f3e262 PS1, Line 279: ${HASH}
""
Done
https://review.coreboot.org/c/coreboot/+/59169/comment/d381e4d6_39efdc1d PS1, Line 285: ${HASH}
""
Done