Skip to content

Conversation

@tukusejssirs
Copy link

@tukusejssirs
Copy link
Author

I’ve just created this PR. I tried to test it (manually) if it works correctly (it should), however, @Lin-Buo-Ren, plese review and test it thoroughly.

Just a question: should we also remove L1079?

Copy link
Member

@brlin-tw brlin-tw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: should we also remove L1079?

Keep it as WoeUSB still requires internet access after the merge in some cases.

@tukusejssirs
Copy link
Author

tukusejssirs commented Nov 19, 2020

Changes to the code:

  1. I added $uefi_ntfs_img_download variable that should contain Boolean string. If set to false, it will use the supplied image (if it is a file), else it will download the UEFI NTFS image. By default, it is set to true on line 65.

  2. Also, I used the name refs where necessary, as you required.

  3. I move the $uefi_ntfs_img test to the check_runtime_dependencies() function, as you required. Note that I have simplified the condition (a bit) using grep (line 702), however, I could not find a better solution to check the -u/--uefi-ntfs-img then using a for cycle, because $uefi_ntfs_img might contain spaces. I don’t like the for cycle here, because it increases the overhead and depends on seq. If you know of a better way, let me know please.

@tukusejssirs
Copy link
Author

@Lin-Buo-Ren, is there anything that needs to be resolved (besides rebasing) before merging this PR? 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an option to provide path to uefi-ntfs.img

2 participants