Skip to content

Conversation

@alexandre-abrioux
Copy link
Contributor

@alexandre-abrioux alexandre-abrioux commented Oct 19, 2021

This fixes #60 and takes into consideration the baseURL parameter of an axios instance.

Copy link
Owner

@hg-pyun hg-pyun left a comment

Choose a reason for hiding this comment

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

Thank you for PR. I will review on weekend.

@alexandre-abrioux
Copy link
Contributor Author

Just forgot to update error.ts, it's done now :)

.makeDateFormat(new Date())
.makeMethod(method)
.makeUrl(url)
.makeUrl(stringBuilder.combineURLs(baseURL, url))
Copy link
Owner

@hg-pyun hg-pyun Oct 29, 2021

Choose a reason for hiding this comment

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

MakeUrl is an abstract method of logic that manages url. I think the combine URLs business logic should be inside makeUrl.

How about .makeUrl(url, baseUrl)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! done :)

Copy link
Owner

Choose a reason for hiding this comment

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

Good. I will test and merge. Thank you.

@hg-pyun hg-pyun merged commit 258ad24 into hg-pyun:master Nov 5, 2021
@hg-pyun hg-pyun mentioned this pull request Nov 5, 2021
3 tasks
@alexandre-abrioux alexandre-abrioux deleted the feat-baseUrl branch November 5, 2021 08:45
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.

formatting is not as advertized

2 participants