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

Add support to localhost urls while debugging #3664

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,11 @@ export class Container {

@memoize()
private get baseGkDevUri(): Uri {
if (this.prereleaseOrDebugging) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest instead of this.prereleaseOrDebugging, to allow the value local for gitkraken.env and then checking that here.

And if it is local and this url is not provided, we should throw because we don't want someone targeting prod and thinking they were on a local setup.

Same with the other cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started with that local value for env, but if I do that and you only want to overwrite 1 URL, you still have to manually put the other ones because env won't tell you anymore if you're on dev, staging or prod. Which is totally okay if we want to do that and make the developer aware they have to manually set all URLs in config. In fact, my first commit does that, and I could retake it if we want to go down this path and throw instead of setting a default URL, as you said.

However, I found it unconvenient that you had to manually set all URLs if you wanted to overwrite just one, so I switched to a useLocal boolean that would "enable" the use of URLs, but again it seemed like setting an URL should be enough to specify that it should use it. With that, you would overwrite the desired URL, and leave all the rest up to env to decide where to point at.

Again, I'm totally up for both options so I appreciate the point of view

const url: string | undefined = configuration.getAny('gitkraken.url.gkdev.base');
if (url) return Uri.parse(url);
}

if (this.env === 'staging') {
return Uri.parse('https://staging.gitkraken.dev');
}
Expand Down
11 changes: 11 additions & 0 deletions src/plus/gk/serverConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { memoize } from '../../system/decorators/memoize';
import { Logger } from '../../system/logger';
import type { LogScope } from '../../system/logger.scope';
import { getLogScope } from '../../system/logger.scope';
import { configuration } from '../../system/vscode/configuration';

interface FetchOptions {
cancellation?: CancellationToken;
Expand All @@ -47,6 +48,11 @@ export class ServerConnection implements Disposable {

@memoize()
private get baseApiUri(): Uri {
if (this.container.prereleaseOrDebugging) {
const url: string | undefined = configuration.getAny('gitkraken.url.api');
if (url) return Uri.parse(url);
}

if (this.container.env === 'staging') {
return Uri.parse('https://stagingapi.gitkraken.com');
}
Expand All @@ -64,6 +70,11 @@ export class ServerConnection implements Disposable {

@memoize()
private get baseGkDevApiUri(): Uri {
if (this.container.prereleaseOrDebugging) {
const url: string | undefined = configuration.getAny('gitkraken.url.gkdev.api');
if (url) return Uri.parse(url);
}

if (this.container.env === 'staging') {
return Uri.parse('https://staging-api.gitkraken.dev');
}
Expand Down
Loading