[typescript][angular] basePath through injected config#367
Conversation
…r, create test cases for angular 6
macjohnny
left a comment
macjohnny
left a comment
There was a problem hiding this comment.
that's just awesome. This will greatly help to keep the code quality by avoiding regressions.
Looks good to me.
| <argument>--progress=false</argument> | ||
| <argument>--no-watch</argument> | ||
| <argument>--browsers</argument> | ||
| <argument>ChromeHeadless</argument> |
There was a problem hiding this comment.
glad it worked with ChromeHeadless
|
@wing328 any idea where to put a note about the |
|
@macjohnny : The correct place would probably be the Testing section in contributing.md: https://github.com/OpenAPITools/openapi-generator/blob/master/CONTRIBUTING.md#testing The sample there is listing how it works for ruby, but angular is equivalent. Run either Run either Alternatively, for just angular, |
|
@akehir you are right. Would you mind updating your PR and adding it to the contributing.md file? |
Shall we document that in the wiki page instead? If we document the test procedure for other generators in the contributing guideline, the page will become too large and likely potential contributors will just skip it. |
|
@wing328 @macjohnny : Where's the wiki - here on github? If you point me to the correct location, I'll add the information there. An alternative is that we create a readme.md / contributing.md within the typescript-angular template folder itself. |
|
@wing328 the wiki repository https://github.com/OpenAPITools/openapi-generator.wiki.git does not seem to be public. |
|
@macjohnny, yes it is. To read it the URL is https://github.com/OpenAPITools/openapi-generator/wiki To clone it the URL you have provided works: (without authentication) |
|
@jmini how can you file a PR to change the wiki? |
|
@wing328 could you add this to the 3.0.3 milestone? |
|
cc the TS technical committee: @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) |
|
I've just restarted the CircleCI job. If it passes, I'll merge this PR into master. |
…r, create test cases for angular 6 (OpenAPITools#367)
5 participants
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.master,3.1.x,4.0.x. Default:master.Description of the PR
@macjohnny : This resolves #282 , implementing my swagger-codegen pull request on the fork.
I notice, that there are a few tests missing, notably the ones I've previously added for angular 6.
CI is here: https://travis-ci.org/akehir/openapi-generator
The original PR here: swagger-api/swagger-codegen#8305
Original description of the PR
Instead of using the this.basepath, we use this.configuration.basepath - allowing us to inject a different configuration (after loading a configuration file over http during APP_INITIALIZER).
We've been using this setup at a project for a while and it has been working well for us (different api url's in different environments, provided by the webserver on runtime).
This would close the problems discussed here: #3221 (comment) and #7949.
This the distinction to the existing PR #7965 is, that it is not required to generate / handle an observable, but rather just by changing the injected configuration, another basePath would be used. Do you think this would solve your concerns @chrismellard?
Since the rest of the swagger api client is always referring to the this.configuration.{...}, I believe this approach might be more consistent with the general handling of the configuration service.
To keep compatibility with implementations that don't inject a configuration, but just the base path, I set the base path on the internal configuration object to the injected basepath (when they don't exist).
This is the new PR for #289