Skip to content

[android] - make changing base url a part of public API.#245

Merged
tobrun merged 1 commit into
masterfrom
244-make-changing-base-url-public-api
Dec 20, 2016
Merged

[android] - make changing base url a part of public API.#245
tobrun merged 1 commit into
masterfrom
244-make-changing-base-url-public-api

Conversation

@tobrun

@tobrun tobrun commented Dec 20, 2016

Copy link
Copy Markdown
Member

Closes #244.

@tobrun tobrun self-assigned this Dec 20, 2016
@tobrun tobrun requested a review from cammace December 20, 2016 10:18
@mention-bot

Copy link
Copy Markdown

@tobrun, thanks for your PR! By analyzing the history of the files in this pull request, we identified @cammace, @ivovandongen and @zugaldia to be potential reviewers.

* Get the base url of the API.
*
* @return the base url used as endpoint.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing @SInCE 2.0.0 tag


/**
* Set the base url of the API.
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing @SInCE 2.0.0 and @param tags

return new MapboxDirections(this);
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No need to expose this in Directions v4, it has been deprecated.

return this;
}

public Builder setBaseUrl(String baseUrl) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javadoc? Looks like we are also missing javadoc for the method above: setClientAppName

return this;
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javadoc

return this;
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javadoc

return this;
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javadoc

return this;
}

@Override

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javadoc

@cammace

cammace commented Dec 20, 2016

Copy link
Copy Markdown

Also, build is failing since we are now using 5.0.0 snapshot correct?

@tobrun

tobrun commented Dec 20, 2016

Copy link
Copy Markdown
Member Author

thanks for the review, the build is failing since 4.2.0 snapshot doesn't exist anymore -> that is why I created #247. IMO we should avoid using snapshots between projects except for doing some testing.

@tobrun tobrun force-pushed the 244-make-changing-base-url-public-api branch from 6e357ea to b0432db Compare December 20, 2016 19:55

@cammace cammace left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👌

@tobrun tobrun merged commit 4e710b1 into master Dec 20, 2016
@tobrun tobrun deleted the 244-make-changing-base-url-public-api branch December 20, 2016 20:20
@zugaldia zugaldia mentioned this pull request Feb 10, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Feb 22, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 9, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 17, 2017
9 tasks
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.

3 participants