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

feat(help): Skip version checks if it will cause permission prompts #663

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

annervisser
Copy link
Contributor

feat(help): Skip version checks if it will cause permission prompts

When a user hasn't granted net permissions to the default upgrade provider,
they would be prompted to allow it when running the help/--help command.

This commit adds an extra method to Provider where a provider should check
any permissions it needs to return available versions have been pre-granted.

If the necessary permissions have NOT been granted, the help command will
skip checking for newer versions. The upgrade command is not affected.

BREAKING CHANGE: Custom Provider implementations will have to implement hasRequiredPermissions()

@annervisser annervisser changed the title Feat/help/skip version checks if it will cause permission prompts feat(help): Skip version checks if it will cause permission prompts Oct 15, 2023
Copy link
Owner

@c4spar c4spar left a comment

Choose a reason for hiding this comment

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

Thx for you pr @annervisser 👍 LGMT!

command/upgrade/_check_version.ts Show resolved Hide resolved
command/upgrade/provider.ts Show resolved Hide resolved
@annervisser
Copy link
Contributor Author

@c4spar anything I need to do to get those checks to run?

@c4spar
Copy link
Owner

c4spar commented Dec 4, 2023

Normally this should happen automatically. Looks like GitHub actions has a hickup. Maybe try to push something and then revert it.

@annervisser annervisser force-pushed the feat/help/skip-version-checks-if-it-will-cause-permission-prompts branch from d9d6a6a to d406108 Compare December 5, 2023 10:27
@annervisser
Copy link
Contributor Author

@c4spar
Copy link
Owner

c4spar commented Dec 6, 2023

thx @annervisser! i will merge it when i prepare the 1.0 release.

@annervisser annervisser force-pushed the feat/help/skip-version-checks-if-it-will-cause-permission-prompts branch 2 times, most recently from 504ba12 to f54d4d7 Compare February 6, 2024 09:56
@annervisser
Copy link
Contributor Author

@c4spar rebased onto main

@c4spar
Copy link
Owner

c4spar commented Feb 18, 2024

@annervisser, i noticed we could also implement this without a breaking change by using a try catch block and checking for a PermissionDenied error. What do you think?

@annervisser
Copy link
Contributor Author

@annervisser, i noticed we could also implement this without a breaking change by using a try catch block and checking for a PermissionDenied error. What do you think?

@c4spar That wouldn't stop Deno from prompting the user for permissions. My main motivation for this PR was that I disliked being prompted for network access when simply running my-cli-tool help

@annervisser annervisser force-pushed the feat/help/skip-version-checks-if-it-will-cause-permission-prompts branch from 855e144 to 75b5275 Compare October 13, 2024 17:23
When a user hasn't granted net permissions to the default upgrade provider,
they would be prompted to allow it when running the `help`/`--help` command.

This commit adds an extra method to `Provider` where a provider should check
any permissions it needs to return available versions have been pre-granted.

If the necessary permissions have NOT been granted, the help command will
skip checking for newer versions. The upgrade command is not affected.

BREAKING CHANGE: Custom `Provider` implementations will have to implement `hasRequiredPermissions()`
@annervisser annervisser force-pushed the feat/help/skip-version-checks-if-it-will-cause-permission-prompts branch from 75b5275 to 657c39b Compare December 1, 2024 13:19
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