search

BenWestgate / Bails

Bails is a Bitcoin solution protecting against surveillance, censorship, and confiscation. It installs Bitcoin Core to Tails encrypted Persistent Storage, creates and recovers Bitcoin Core wallets from Codex32 (BIP93) seed backups, and creates backup Bails USB sticks and shareable blank Bails USB sticks. Learn more in README.md.
MIT License
45 stars 8 forks source link

Resolve shellcheck lint issues #100

Closed epiccurious closed 4 months ago

epiccurious commented 6 months ago

This refactor will require testing. Keeping in Draft for now.

epiccurious commented 6 months ago

@BenWestgate please see my responses to your questions here and here on my fork.

epiccurious commented 6 months ago

Resolves #95.

epiccurious commented 6 months ago

Here are the four ShellCheck rules that are excluded in the GitHub Actions lint:

epiccurious commented 5 months ago

@BenWestgate To make it easier to do testing, can you please enable this setting at bottom of the main Settings page?

Screenshot

epiccurious commented 5 months ago

Rebased.

epiccurious commented 5 months ago

I'm seeing an issue with the PR during testing. Something is broken with the clone/download process. Will post a screen recording showing the failure.

epiccurious commented 5 months ago

Here's the video recording for the failure: Screencast from 2024-05-03 11-35-22.webm

Here's the video recording for Bails master branch succeeding: Screencast from 2024-05-03 11-47-02.webm

PR isn't ready to merge yet unfortunately.

BenWestgate commented 4 months ago

@BenWestgate To make it easier to do testing, can you please enable this setting at bottom of the main Settings page?

Screenshot

It already is: image

BenWestgate commented 4 months ago

Here's the video recording for the failure: Screencast from 2024-05-03 11-35-22.webm

Here's the video recording for Bails master branch succeeding: Screencast from 2024-05-03 11-47-02.webm

PR isn't ready to merge yet unfortunately.

So the download for the signatures threw an error, you can see that in the terminal and that's the Zenity message as well.

Did you look up what that error meant?

Did you try again starting fresh?

It may have been a once off download failure, this PR does not touch the code that downloads the signatures so Master is the same. Which means this error is non-blocking unless you always got it.

epiccurious commented 4 months ago

It may have been a once off download failure

You were right. I re-ran it again on the same branch and it's downloading fine. That's the first time I've seen that intermittent failure. Perhaps a wget command that could use a retry=5 argument and/or better error handling.

Setting PR as ready to review. Ready to merge.

BenWestgate commented 4 months ago

Perhaps a wget command that could use a retry=5 argument and/or better error handling.

The default is to retry 20 times, with the exception of fatal errors like “connection refused” or “not found” (404), which are not retried.

How would you suggest the error handling be improved? Just remove the option to try again? Since it already tried many times in order to fail there in the first place?