Skip to content

fix: change --update flag to --update-cache in docs and tests#503

Merged
josegonzalez merged 3 commits intogliderlabs:masterfrom
mtgto:fix_usage_update
Feb 27, 2020
Merged

fix: change --update flag to --update-cache in docs and tests#503
josegonzalez merged 3 commits intogliderlabs:masterfrom
mtgto:fix_usage_update

Conversation

@mtgto
Copy link
Contributor

@mtgto mtgto commented Mar 11, 2019

apk --update flag is really --update-cache.

https://github.com/alpinelinux/apk-tools/blob/v2.10.3/src/apk.c#L201

Apk uses getopt_long (3),
https://github.com/alpinelinux/apk-tools/blob/v2.10.3/src/apk.c#L574

So, --update flag is only abbreviated from --update-cache by getopt_long.

Long option names may be abbreviated if the abbreviation is unique or
is an exact match for some defined option.

https://linux.die.net/man/3/getopt_long

@finefoot
Copy link

@josegonzalez Friendly ping. :) If possible it would be nice if you could leave a comment.

I stumbled upon this issue as well, wondering what the --update argument is, when in fact it's just the abbreviation for --update-cache. The official short argument is -U, so I think it's best practice to either use that or the full --update-cache, right? Furthermore, the congruity of --update with --update-cache via getopt_long might get lost with possible future arguments called --update....

@josegonzalez
Copy link
Member

If this gets rebased and tests pass, I can merge.

@mtgto
Copy link
Contributor Author

mtgto commented Feb 25, 2020

@josegonzalez okay, i'll fix it.

@josegonzalez
Copy link
Member

Make sure to comment here again when you do - Github won't send notifications on updated PRs and I have a ton of repos that make PR watching kinda not possible :(

@mtgto
Copy link
Contributor Author

mtgto commented Feb 25, 2020

@josegonzalez Fixed.
alpine has also v3.10 and 3.11, but I don't create test files for these versions.
https://hub.docker.com/_/alpine

FROM gliderlabs/alpine:3.3

RUN apk add --update \
RUN apk add --update-cache \

Choose a reason for hiding this comment

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

Perhaps instead use the --no-cache option, which installs the package without creating a local cache; with that, it's also not needed to rm -rf /var/cache/apk/* afterwards

RUN apk add --no-cache \
      python \
      python-dev \
      py-pip \
      build-base \
   && pip install virtualenv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's also not needed to rm -rf /var/cache/apk/* afterwards

I think so.

However this PR does not responsible for it, IMO.

Copy link
Member

Choose a reason for hiding this comment

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

Do either of you mind going in and making a PR with the above change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josegonzalez Sorry, I have no motivation.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, its all OSS so if folks have time to fix, thats great, if not, thats cool too :)

@josegonzalez josegonzalez merged commit 25d285f into gliderlabs:master Feb 27, 2020
@mtgto mtgto deleted the fix_usage_update branch February 27, 2020 23:21
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.

4 participants