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

[BUG FIX] Remove ngrok version check everytime a op plugin command is called #474

Merged
merged 8 commits into from
Sep 25, 2024

Conversation

MOmarMiraj
Copy link
Contributor

@MOmarMiraj MOmarMiraj commented Jul 10, 2024

Overview

This MR will fix a bug regarding ngrok --version being called on every op plugin command.

Type of change

  • Created a new plugin
  • Improved an existing plugin
  • Fixed a bug in an existing plugin
  • Improved contributor utilities or experience

Related Issue(s)

  • Resolves: #
  • Relates: #

How To Test

The test pipelines should pass as usual and when running an op plugin command, if you check the activity monitor.. there should be no ngrok processes. (Note: Must have the ngrok CLI installed to test this)

Sometimes its hard to see the ngrok process on the activity monitor/task manager. An easier way to test is by changing L129 on plugins/ngrok/provisioner.go to open up any application you have on your PC and see if the app opens up when you run a op plugin command.

Changelog

The op plugin commands do not call a ngrok version check even if the ngrok plugin is not initialized.

@MOmarMiraj MOmarMiraj added the bug Something isn't working label Jul 10, 2024
@MOmarMiraj MOmarMiraj self-assigned this Jul 10, 2024
Copy link
Contributor

@AndyTitu AndyTitu left a comment

Choose a reason for hiding this comment

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

lgtm apart from the wording suggestion

@MOmarMiraj MOmarMiraj force-pushed the omar/fix-ngrok-version-check branch 3 times, most recently from f87e9af to 20ca75f Compare August 1, 2024 13:24
@1Password 1Password deleted a comment from github-actions bot Aug 1, 2024
@1Password 1Password deleted a comment from github-actions bot Aug 1, 2024
@MOmarMiraj MOmarMiraj requested a review from hculea August 15, 2024 15:44
Copy link
Contributor

⚠️ This PR contains unsigned commits. To get your PR merged, please sign those commits (git rebase --exec 'git commit -S --amend --no-edit -n' @{upstream}) and force push them to this branch (git push --force-with-lease).

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

  1. Generate a GPG key
  2. Add the GPG key to your GitHub account
  3. Configure git to use your GPG key for commit signing
Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

  1. Generate an SSH key and add it to ssh-agent
  2. Add the SSH key to your GitHub account
  3. Configure git to use your SSH key for commit signing
Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.

Learn how to use 1Password to sign your commits.

Watch the demo

mrjones2014 and others added 6 commits September 24, 2024 15:51
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.17.0 to 0.23.0.
- [Commits](golang/net@v0.17.0...v0.23.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Omar Miraj <[email protected]>
Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Code looks good, and the changes make sense - calling the ngrok --version command only when actually provisioning and not in the Provisioner's instantiation.

Haven't functionally tested.

@MOmarMiraj MOmarMiraj merged commit 36e4bf4 into main Sep 25, 2024
5 checks passed
@MOmarMiraj MOmarMiraj deleted the omar/fix-ngrok-version-check branch September 25, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants