-
Notifications
You must be signed in to change notification settings - Fork 22
add basePath to GenApiClient #10
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 basePath to GenApiClient #10
Conversation
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.
Great work! Please fix the requested changes.
@@ -2,5 +2,6 @@ export 'requests.dart'; | |||
export 'params.dart'; | |||
|
|||
class GenApiClient { | |||
const GenApiClient(); | |||
final String basePath; |
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.
Just path
should be enough?
@@ -2,5 +2,6 @@ export 'requests.dart'; | |||
export 'params.dart'; | |||
|
|||
class GenApiClient { | |||
const GenApiClient(); | |||
final String basePath; | |||
const GenApiClient([this.basePath = ""]); |
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.
Please make basePath
named optional argument.
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.
Do you mean just initialize it to null instead ?
retrofit_gen/lib/writer/writer.dart
Outdated
@@ -22,7 +22,7 @@ class Writer { | |||
|
|||
sb.write('var req = base.${r.method}'); | |||
|
|||
if (r.path != null) sb.write('.path("${r.path}")'); | |||
if (r.path != null) sb.write('.path("\$basePath${r.path}")'); |
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.
Use two different calls to .path
method. That way, Jaguar Resty will take care of missing forward slashes, etc.
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.
ho ok !
@jaumard Please fix the merge conflicts. |
Let me know if there something more to do @tejainece should be ok now |
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.
Please fix the requested changes.
@@ -2,5 +2,6 @@ export 'requests.dart'; | |||
export 'params.dart'; | |||
|
|||
class GenApiClient { | |||
const GenApiClient(); | |||
final String path; | |||
const GenApiClient([this.path = null]); |
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.
Please change it to named optional parameter.
const GenApiClient({this.path});
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.
Done :)
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.
LGTM
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.
LGTM
fix #1