Skip to content

Simplify+optimize - #856

Merged
flexiondotorg merged 28 commits into
wimpysworld:mainfrom
kstr0k:simplify+optimize
Oct 19, 2023
Merged

flexiondotorg merged 28 commits into
wimpysworld:mainfrom
kstr0k:simplify+optimize

Conversation

@mralusw

@mralusw mralusw commented Jul 23, 2023

Copy link
Copy Markdown
Contributor

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).

mralusw added 27 commits July 17, 2023 18:48
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

mralusw commented Jul 23, 2023

Copy link
Copy Markdown
Contributor Author

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

@philclifford

Copy link
Copy Markdown
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.

Comment thread deb-get
# 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
Copy Markdown
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

@philclifford philclifford left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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

mralusw commented Aug 30, 2023 •

Copy link
Copy Markdown
Contributor Author

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.

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