Attention is currently required from: Thomas Heijligen, Alexander Goncharov.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 )
Change subject: util: add bash completion script ......................................................................
Patch Set 9: Code-Review+1
(5 comments)
Patchset:
PS9: Just some minor issues. Otherwise looks good to me.
File Makefile:
https://review.coreboot.org/c/flashrom/+/68247/comment/548f9631_ce2d77a1 PS9, Line 589: ACTIVE_PROGRAMMERS += internal It took a little until I understood this programmer list is needed for the substitution. Please add a short note in the commit message mentioning that this is added in the Makefile and its purpose.
https://review.coreboot.org/c/flashrom/+/68247/comment/e720674b_819ef42a PS9, Line 1032: $(PROGRAM).8 $(PROGRAM).8.html $(PROGRAM).bash $(BUILD_DETAILS_FILE) One tab too much
https://review.coreboot.org/c/flashrom/+/68247/comment/e391935a_689a6738 PS9, Line 1041: .bash
IIRC there should be no `. […]
That's just the file in the source directory. I think we can't remove the file suffix that easily, else it gets in conflict with the flashrom binary since both have the same same. Though I agree the installed file shouldn't have an suffix anymore.
Actually, I like that the resulting file has a suffix. I suggest using `.bash-completion` for that file lying in the source directory. Then it's consistent with the template file (not two different suffixes). Just remove the suffix for the destination in the install command.
File util/flashrom.bash-completion.tmpl:
https://review.coreboot.org/c/flashrom/+/68247/comment/74b2f75f_06fe7c64 PS9, Line 43: OPTS="--help Missing \ at the end of each line.
-
Felix Singer (Code Review)