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

Simplify+optimize #856

Merged
merged 28 commits into from
Oct 19, 2023
Merged

Simplify+optimize #856

merged 28 commits into from
Oct 19, 2023

Conversation

mralusw
Copy link
Contributor

@mralusw mralusw commented Jul 23, 2023

Hi, I've found deb-get really useful and played with it for a bit.

I've tried to optimize performance (by avoiding repeated external calls and caching some results — e.g. list is about 4 times faster), fix some (minor) bugs and simplify some shell constructs (e.g. replacing grep | cut with sed ...), fix some shellcheck warnings etc.

I've tried to make the diffs between commits as small / clear as possible (as opposed to a big rewrite), hence the number of commits.

I've tested it as much as I could on my normal workflows, hopefully github-actions can also help here (I'm not sure how to run testing on my fork).

use simpler constructs, avoid calls to external apps, compact some
statements to reduce debugging (-x) output
Also remove exports only performed with --quiet; preparing for `eval`
Could be installed but not configured
Set trap to ensure lock removal; warn if cache locked
@mralusw
Copy link
Contributor Author

mralusw commented Jul 23, 2023

Hm, I got Error: Resource not accessible by integration... Not sure what that means

@philclifford
Copy link
Member

Thanks for this excellent and instructive work. Don't worry about the error you mentioned: the actions seem to be broken lately and need some attention and revision.
I've tested this PR lightly with normal update/upgrade cycles on Ubuntu-Mate (with no problems other than normal transient upstream issues). I'd like to get time to run some github-actions testing across a small matrix of distros but that work (and testing and fixing the tests) is beyond my available bandwidth for a while.
To run github-actions on your fork you just need to enable it in the settings of the fork , but first you might want to tweak the actions definitions in your fork/branch and also possibly ensure those tweaks don't accidentally get added to your PR.

@@ -471,18 +440,17 @@ function validate_deb() {
# If the method is github and the cache file is empty, ignore the package
# The GitHub API is rate limit has likely been reached
if [ "${METHOD}" == github ] && [ ! -s "${CACHE_FILE}" ]; then
if [ "${ACTION}" != "prettylist" ] && [ "${ACTION}" != "remove" ] && [ "${ACTION}" != "purge" ]; then
if ! [[ ' prettylist remove purge ' =~ " ${ACTION} " ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

👀

philclifford
philclifford previously approved these changes Aug 30, 2023
Copy link
Member

@philclifford philclifford left a comment

Choose a reason for hiding this comment

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

LGTM
Limited operational testing so far

@philclifford philclifford dismissed their stale review August 30, 2023 02:28

search appears to be broken

@philclifford
Copy link
Member

Looking at things I rarely use part 1: search

(base) ✔ ~/src/deb-get [official|…4⚑ 2] 
03:29 $ ./deb-get search "tri"
lutris
motrix
tribler
trilium
trivy
(base) ✔ ~/src/deb-get [official|…4⚑ 2] 
03:30 $ ./deb-get search "^tri"
tribler
trilium
trivy
(base) ✔ ~/src/deb-get [official|…4⚑ 2] 
03:30 $ git checkout simplify+optimize 
Switched to branch 'simplify+optimize'
(base) ✔ ~/src/deb-get [simplify+optimize L|…4⚑ 2] 
03:30 $ ./deb-get search "^tri"
1password
agena
android-messages-desktop
antimicrox
...
zoom
zotero

@mralusw
Copy link
Contributor Author

mralusw commented Aug 30, 2023

Looking at things I rarely use part 1: search

Yup — last commit (1f4581b), which purported to fix -... patterns, actually broke search completely due to the simultaneous refactoring. Trivial fix. Not sure how that got through — I guess more tests would help.

@flexiondotorg flexiondotorg merged commit ab208bb into wimpysworld:main Oct 19, 2023
2 checks passed
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.

3 participants