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

A few improvements #11

Merged
merged 6 commits into from
May 13, 2020
Merged

A few improvements #11

merged 6 commits into from
May 13, 2020

Conversation

SmartFinn
Copy link
Contributor

The PR adds a few changes that I using in my private fork of this project:

  1. Avoid using GitHub API calls to allow more than 60 request per hour per routable IP
  2. Reduce execution of external commands when it possible
  3. Fixes regex in sed to avoid false matching
  4. Add short forms for options
  5. sdd list shows installed packages if no extra option specified

Copy link
Owner

@pylipp pylipp left a comment

Choose a reason for hiding this comment

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

Thanks a lot! This brings many usability and code readability improvements.
I noticed one failing test, do you mind having a look why? EDIT: not related to your changes, I'll have a look myself :)
You can also extend the Changelog (mention the new short option forms, and the fix) and update the README (section about listing apps).

lib/sdd/framework/utils.bash Show resolved Hide resolved
lib/sdd/framework/utils.bash Outdated Show resolved Hide resolved
lib/sdd/framework/utils.bash Show resolved Hide resolved
@SmartFinn
Copy link
Contributor Author

SmartFinn commented May 13, 2020

@pylipp the changelog updated. In relation to README, I have no idea how to mention the short options except duplicating the commands.

@SmartFinn SmartFinn requested a review from pylipp May 13, 2020 17:10
@pylipp pylipp merged commit 6915c56 into pylipp:master May 13, 2020
@pylipp
Copy link
Owner

pylipp commented May 13, 2020

Thanks again for the contribution! How did you find the project, and what are you using it for?

@SmartFinn
Copy link
Contributor Author

I saw your post on Reddit and I thought that is a good way to replace my zsh functions for installing binaries from GitHub to the separate app.

@SmartFinn SmartFinn deleted the improvements branch May 14, 2020 13:44
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.

2 participants