Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add update option #343

Closed

Conversation

strangelookingnerd
Copy link
Contributor

What does this PR aim to accomplish?:

Add an option to manually update PADD if there is a new version available.

How does this PR accomplish the above?:

Add new -u | --update option that updates the script if there is a new version available using wget or curl.

Link documentation PRs if any are needed to support this PR:

README.md has been updated.


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.

  • I have read the above and my PR is ready for review.

@strangelookingnerd
Copy link
Contributor Author

Please understand that I'm by no means an expert for Shell, so any suggestions are welcome 😄

padd.sh Outdated Show resolved Hide resolved
@yubiuser
Copy link
Member

Please target development branch instead of master

@strangelookingnerd strangelookingnerd changed the base branch from master to development February 27, 2023 20:19
@strangelookingnerd strangelookingnerd requested review from rdwebdesign and yubiuser and removed request for rdwebdesign and yubiuser February 28, 2023 09:19
padd.sh Outdated Show resolved Hide resolved
padd.sh Outdated Show resolved Hide resolved
@strangelookingnerd strangelookingnerd requested review from rdwebdesign and removed request for yubiuser February 28, 2023 19:47
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Signed-off-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
@github-actions
Copy link

Conflicts have been resolved.

@strangelookingnerd strangelookingnerd requested review from yubiuser and removed request for rdwebdesign March 13, 2023 06:56
padd.sh Outdated Show resolved Hide resolved
padd.sh Outdated Show resolved Hide resolved
padd.sh Outdated Show resolved Hide resolved
Co-authored-by: yubiuser <ckoenig@posteo.de>
Signed-off-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
padd.sh Outdated Show resolved Hide resolved
Signed-off-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
padd.sh Show resolved Hide resolved
@yubiuser yubiuser mentioned this pull request Apr 7, 2023
padd.sh Outdated Show resolved Hide resolved
padd.sh Outdated Show resolved Hide resolved
strangelookingnerd and others added 2 commits April 14, 2023 15:23
Co-authored-by: yubiuser <ckoenig@posteo.de>
Signed-off-by: strangelookingnerd <49242855+strangelookingnerd@users.noreply.github.com>
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Currently, the output looks like

pi@s740:~$ ./padd.sh -u
[i] Updates are available
[i] Downloading via wget ...
--2023-04-14 15:52:36--  https://install.padd.sh/
Resolving install.padd.sh (install.padd.sh)... 52.33.207.7, 44.230.85.241
Connecting to install.padd.sh (install.padd.sh)|52.33.207.7|:443... connected.
HTTP request sent, awaiting response... 307 Temporary Redirect
Location: https://raw.githubusercontent.com/pi-hole/PADD/master/padd.sh [following]
--2023-04-14 15:52:37--  https://raw.githubusercontent.com/pi-hole/PADD/master/padd.sh
Resolving raw.githubusercontent.com (raw.githubusercontent.com)... 185.199.110.133, 185.199.108.133, 185.199.111.133, ...
Connecting to raw.githubusercontent.com (raw.githubusercontent.com)|185.199.110.133|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 63631 (62K) [text/plain]
Saving to: '/home/pi/padd.sh'

/home/pi/padd.sh                                       100%[=========================================================================================================================>]  62.14K  --.-KB/s    in 0.02s   

2023-04-14 15:52:37 (2.79 MB/s) - '/home/pi/padd.sh' saved [63631/63631]

[✓] ... done. Restart PADD for the update to take effect

  • I'm not sure if we should keep all the wget (and curl) output or suppress it?
  • Also there is no error handling at the moment if downloading fails for any reason.
  • You might want to show the current and the new version as well?

Add error handling
Show current and update version
@strangelookingnerd
Copy link
Contributor Author

I'm not sure if we should keep all the wget (and curl) output or suppress it?

I decided to suppress the output. Looks cleaner that way. Could still be reverted if you think otherwise.

Also there is no error handling at the moment if downloading fails for any reason.

I added some error handling, let me know what you think.

You might want to show the current and the new version as well?

That is a great idea, added it as well.

@yubiuser
Copy link
Member

Looks nicer now

pi@s740:~$ ./padd.sh -u
[i] Updating PADD from v3.09.1 to v3.10.1
[i] Downloading PADD update via wget ...
[✓] ... done. Restart PADD for the update to take effect

Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

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

Looks good from my side.
Please rebase on latest development and squash all commits down.

@strangelookingnerd strangelookingnerd mentioned this pull request Apr 15, 2023
1 task
@strangelookingnerd
Copy link
Contributor Author

Re-created PR to squash commits in #352

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.

None yet

3 participants