Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix download uefi-ntfs.img #25

Merged
merged 2 commits into from
Feb 27, 2021

Conversation

FuPeiJiang
Copy link
Contributor

@WaxyMocha
Copy link
Member

Ok so I checked this part of code and turns out that in general, I don't think it does anything. You are right that current code downloads html instead of image, but then it is copied to /dev/sdx2 and that doesn't make sense. One more reason to refactor the whole thing.
Thanks for PR :)

@WaxyMocha WaxyMocha merged commit 3572db0 into WoeUSB:master Feb 27, 2021
@WaxyMocha
Copy link
Member

No, wait. If it's just raw image, it does make sense to copy it directly to block device.

@FuPeiJiang
Copy link
Contributor Author

this works
sudo dd if="/path/to/uefi-ntfs.img" of=/dev/sdX2
but it's dd, so it's dangerous,

python does a copy, which is less dangerous than overwrite, but it would require mounting "/path/to/uefi-ntfs.img" AND mounting /dev/sdX2

@FuPeiJiang
Copy link
Contributor Author

I have more commits,

but since: CONTRIBUTING.md

Create commits based on minimal independent changes
Avoid creating commits that do multiple things at once as this will help other developers understand the change history.

I guess pull requests shouldn't contain unrelated stuff, so I started with this one

WaxyMocha added a commit that referenced this pull request Feb 28, 2021
Changes:
 - Close #6, Use new logo created by @ragnilorenzo,
 - Merge #23, Add German translation,
 - Merge #24, Auto unmount source and target,
 - Merge #25, Download efi image instead of html page
 - Add category to desktop shortcut,
 - Many small fixes
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants