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

Allow port in loginUrl #388

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

Allow port in loginUrl #388

wants to merge 1 commit into from

Conversation

unsupo
Copy link

@unsupo unsupo commented Mar 21, 2021

The regex does not allow the : in the match for loginUrl, this prevents a port from being included in the loginUrl.

fails with this sfdxAuthUrl before the change and succeeds after change

force://PlatformCLI::<refreshToken>@devhub.my.localhost.sfdcdev.salesforce.com:6101

The regex does not allow the : in the match for loginUrl, this prevents a port from being included in the loginUrl.
@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @unsupo to sign the Salesforce.com Contributor License Agreement.

@codecov-io
Copy link

Codecov Report

Merging #388 (e7c9689) into main (3e27623) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #388   +/-   ##
=======================================
  Coverage   82.70%   82.70%           
=======================================
  Files          38       38           
  Lines        3261     3261           
  Branches      569      569           
=======================================
  Hits         2697     2697           
  Misses        346      346           
  Partials      218      218           
Impacted Files Coverage Δ
src/authInfo.ts 93.55% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 107070d...e7c9689. Read the comment docs.

@peternhale
Copy link
Contributor

@unsupo Thank you for the contribution. Would you mind adding a unit test to cover this change? Unit tests for the modified function can be found here

Copy link
Collaborator

@amphro amphro left a comment

Choose a reason for hiding this comment

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

Test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants