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.

Sign up for GitHub

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

Jump to bottom

updater_overhaul #541

Merged
merged 3 commits into from Nov 19, 2018
Merged

updater_overhaul #541

merged 3 commits into from Nov 19, 2018

Conversation

overdodactyl
Copy link
Contributor

In large part a restructuring/re-write of the last version.

  • Uses perl as a last resort if curl and wget are not available (fixes bash script #537)
  • Aborts and notifies user if none of the above are installed
  • Better use of functions
  • When version numbers are checked, the contents are immediately saved to a temp dir. This allows us to skip using wget/curl/perl a second time
  • Improved messages for users
  • Added various font colors for ease of use and aesthetics

Pinging @claustromaniac and @earthlng ... any feedback/opinions would be great!

@overdodactyl
Copy link
Contributor Author

Screenshot of new printing:

updater

@bogachenko
Copy link

it's awesome

@bogachenko bogachenko mentioned this pull request Nov 19, 2018
Closed
@earthlng
Update updater.sh
c12f6cd 
- by moving `cd "$ff_profile"` to the Execute section we don't need to restore working directory if we hit the Exit 0 in File Handling
- return 1 in confirmation() instead of exit and chained with `&& update_userjs` is functionally identical but always cleans up userjs_temps and restores the original working directory
@earthlng
Copy link
Contributor

wow it looks great, both visually and code-wise.
Please review my commit and let me know if it makes sense and if you're happy with it.

I think you should also move mkdir -p userjs_backups into backup_file() because currently when the folder doesn't exist and update_updater() calls backup_file() the move command will probably fail (right?)

@overdodactyl
Copy link
Contributor Author

Commit looks great, thanks! I moved the location of mkdir -p userjs_backups and made a few more changes, but nothing that should impact functionality.

@earthlng
Copy link
Contributor

okay then, ready to merge. Thanks @overdodactyl

@earthlng earthlng merged commit 106f46d into arkenfox:master Nov 19, 2018
@claustromaniac
Copy link
Contributor

Snazinessssssssssssssssssss!

I second bogachenko: this is awesome. Way more elegant than anything I'd usually write TBH 😆

@Thorin-Oakenpants
Copy link
Contributor

Screenshot of new printing:

Pants Romance 💋 you know what I'm talking about

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bash script
5 participants
@overdodactyl @bogachenko @earthlng @claustromaniac @Thorin-Oakenpants