#2857 PR merged: Insert '$pxe_link_prefix' also "if has_binary gethostip" in the IP case

Labels: cleanup, fixed / solved / done, minor bug

jsmeix opened issue at 2022-09-07 12:25:

  • Type: Bug Fix

  • Impact: Normal

  • Reference to related issue (URL):
    https://github.com/rear/rear/pull/2851

  • How was this pull request tested?
    Not at all tested by me because I do not use PXE

  • Brief description of the changes in this pull request:

In output/PXE/default/810_create_pxelinux_cfg.sh
insert missing '$pxe_link_prefix' for the IP case
also for the "if has_binary gethostip" part, see
https://github.com/rear/rear/pull/2851#issuecomment-1237939248

jsmeix commented at 2022-09-07 12:41:

@gdha @pcahyna
I would also like to clean up the indentation in
output/PXE/default/810_create_pxelinux_cfg.sh
and likely some more "cosmetic" code changes.

Personally I would "just do it by the way"
together with this pull request.
Would this be a problem for you?

@pcahyna
I think I remember that you prefer separated
pull requests for "cosmetic" code changes
which is clearly the more accurate procedure
from a revision control system point of view.

In contrast usually I do "cosmetic" code changes
"just by the way" while I am working on some code
because separated pull requests for "cosmetic" changes
is duplicate administrative work (two times editing same files
plus two times the revision control system related work)
but usually I don't find time for anything more than
what is really needed so usually I would not do any
"cosmetic" changes as a separated additional task.

I think what could be a good compromise for me
are "cosmetic" changes as a separated commits
but all together within one same pull request.

gdha commented at 2022-09-07 14:07:

@gdha @pcahyna I would also like to clean up the indentation in output/PXE/default/810_create_pxelinux_cfg.sh and likely some more "cosmetic" code changes.

Personally I would "just do it by the way" together with this pull request. Would this be a problem for you?

@jsmeix No problem at all.

jsmeix commented at 2022-09-12 12:40:

@gdha @pcahyna
I cleaned up output/PXE/default/810_create_pxelinux_cfg.sh via
https://github.com/rear/rear/pull/2857/commits/a634c44d679a7005fa3585862d009d879c32bf10
so I would appreciate it if you could have a look (as time permits)
if you spot obvious mistakes because of my cleanup changes.

jsmeix commented at 2022-09-13 10:38:

@pcahyna @rear/contributors
when there are no objections
I would like to merge it tomorrow afternoon

[Export of Github issue for rear/rear.]