Skip to content

feat: Revamped setting base URLs - #1042

Merged
antusus merged 2 commits into
mainfrom
sdk-2124_base_urls
Apr 22, 2022
Merged

antusus merged 2 commits into
mainfrom
sdk-2124_base_urls

Conversation

@antusus

@antusus antusus commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

No description provided.

/**
* Default authorization URL
*/
protected static final String DEFAULT_AUTHORIZATION_URL = "https://account.box.com/api/";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we should change this to DEFAULT_BASE_AUTHORIZATION_URL as this is only some part of the URL?

private String baseURL;
private String baseUploadURL;
private String baseAppURL;
private String authorizationURL;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe change this to baseAuthorizationURL as we have below getAuthorizationURL() method which use this as a base URL>

}

/**
* Gets the base URL that's used when sending requests to the Box API. The default value is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we should mention that it returns API_VERSION as well?
Just to inform that after invoke setBaseURL getBaseURL will return a different value.

.add("maxRetryAttempts", this.getMaxRetryAttempts())
.add("subjectType", this.subjectType)
.add("subjectId", this.subjectId);
JsonObject state = Json.parse(super.save()).asObject();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@@ -337,7 +355,7 @@ public void setBaseURL(String baseURL) {
* @return the base upload URL.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as base URL, maybe we should mention that it returns API_VERSION as well?

@arjankowski arjankowski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@antusus
antusus merged commit 129baf7 into main Apr 22, 2022
@antusus
antusus deleted the sdk-2124_base_urls branch April 22, 2022 07:23
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