-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: disable analytics & sentry when fedramp [HEAD-677] #245
chore: disable analytics & sentry when fedramp [HEAD-677] #245
Conversation
Snyk.VisualStudio.Extension.Shared/Settings/SnykGeneralOptionsDialogPage.cs
Outdated
Show resolved
Hide resolved
Snyk.VisualStudio.Extension.Shared/Settings/SnykGeneralOptionsDialogPage.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggestions
@@ -14,6 +14,8 @@ public interface ISnykOptions | |||
/// </summary> | |||
AuthenticationToken ApiToken { get; } | |||
|
|||
bool IsFedramp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually when you have FooType GetFoo()
, it's idiomatic to use a getter only auto-property, so FooType Foo { get; }
public bool IsFedramp() | ||
{ | ||
var endpoint = this.customEndpoint; | ||
if (endpoint.IsNullOrEmpty()) | ||
{ | ||
return false; | ||
} | ||
|
||
var endpointUri = new Uri(endpoint); | ||
return endpointUri.Host.ToLower().EndsWith("snykgov.io"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Suggestion)
You can also do something like this:
public bool IsFedramp() | |
{ | |
var endpoint = this.customEndpoint; | |
if (endpoint.IsNullOrEmpty()) | |
{ | |
return false; | |
} | |
var endpointUri = new Uri(endpoint); | |
return endpointUri.Host.ToLower().EndsWith("snykgov.io"); | |
} | |
public bool IsFedramp() => > Uri.TryCreate(this.customEndpoint, UriKind.Absolute, out var uri) && | |
uri.Host.ToLower().EndsWith("snykgov.io"); |
It's shorter and handles any error in the URI parsing (like invalid format). I recently added code to the CustomEndpoint
setter to avoid invalid URIs, but it's better to be safe anyway.
Description
Provide description of this PR and changes, if linked Jira ticket doesn't cover it in full.
Checklist
Screenshots / GIFs
Visuals that may help the reviewer. Please add screenshots for any UI change. GIFs are most welcome!