maximbaz / arch-secure-boot

UEFI Secure Boot for Arch Linux + btrfs snapshot recovery
ISC License
126 stars 8 forks source link

Remove forward slash to make echos nicer #12

Closed ShellCode33 closed 1 year ago

ShellCode33 commented 1 year ago

Hey ! A really stupid PR because I'm a fussy nerd

image

Thanks for the AUR package btw ;)

maximbaz commented 1 year ago

Haha sure thing, thank you for the contributions! :grin: I think this would break add-efi though, could you check? Maybe for simplicity we can add the slash on line 115, i.e. entry="/$EFI/$NAME.efi" ?

ShellCode33 commented 1 year ago

I does indeed, nice catch ! This new commit should fix the issue.

image

I would totally understand if you don't want to bother adding 2 commits just for the sake of removing a slash duplicate which is not even harmful :sweat_smile:

I didn't notice the fix you suggested (the entry="/$EFI/$NAME.efi"), it works too

maximbaz commented 1 year ago

Thanks for testing! And don't worry at all, I want aaaaaalllllllll the commits you have :grin: :grin: I have a small preference for my suggestion, because the magic in efibootmgr call is already unreadable enough :sweat_smile: If you could push that I'd appreciate, otherwise I'll just merge later and update, and cut a new release :stuck_out_tongue:

ShellCode33 commented 1 year ago

The reason why I didn't use your approach is because the next statement would evaluate to [ -f "/efi//EFI/arch" ], doubling the slash again. But this is not being echoed, so I guess you're right, readability should be prefered. Commit is on its way.

https://github.com/maximbaz/arch-secure-boot/blob/1fd8e9e933519af612f78aeb7f14a416f23912b6/arch-secure-boot#L115-L116

ShellCode33 commented 1 year ago

Done, I forced push to prevent polluting your git history, do not reproduce at ~ :stuck_out_tongue_winking_eye:

maximbaz commented 1 year ago

Your reasoning makes sense, I didn't spot that bit! Agree with you that we should nevertheless prefer the readability :+1:

Thanks a ton!