Skip to content
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

FM-246 Add Swagger SDK generation backend implementation #156

Merged
merged 1 commit into from Jun 29, 2018

Conversation

Projects
None yet
3 participants
@eunice18
Copy link
Contributor

eunice18 commented Jun 18, 2018

No description provided.

@harsha89

This comment has been minimized.

Copy link
Contributor

harsha89 commented Jun 18, 2018

overall please add more debug logs where it necessary

Class<DiagnosticReport> fhirResource,
ICriterion<ReferenceClientParam> where,
ICriterion<TokenClientParam> and) {
Class<DiagnosticReport> fhirResource,

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

exceeds maximum length of line 120

@@ -68,8 +68,8 @@ public static Bundle searchWhereReferenceAndToken(String serverBase,
}

public static Bundle searchWhereReference(String serverBase,
Class<DiagnosticReport> fhirResource,
ICriterion<ReferenceClientParam> where) {
Class<DiagnosticReport> fhirResource,

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

same here


public SwaggerCodeGenerator() {
//Supported SDK languages
sdkLanguages.put("java", "io.swagger.codegen.languages.JavaClientCodegen");

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

are we only supports these languages only? I thought according to the discussion we are supporting all available languages. Can you please check


/**
* Generates the swagger SDK of given language
*

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

parameter descriptions missing

tempSdkGenDir = Files.createTempDirectory(ZIP_ARCHIVE_NAME_PREFIX + "_" + language + "_");

//Create a temporary file to store the swagger definition
swaggerDefFile =Files.createTempFile(tempSdkGenDir,

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

formatting issue

@@ -0,0 +1,117 @@
package org.openmrs.module.fhir.swagger.utils;

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

licence header is missing


/**
* This method used to delete the directory that created during the sdk generation
*

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

parameter descriptions missing


try (FileOutputStream fileOutputStream = new FileOutputStream(archiveLocation + File.separator + archiveName
+ ".zip");
ZipOutputStream zipOutputStream =new ZipOutputStream(fileOutputStream)) {

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

formatting issue space needed after =

*
* @throws IOException if something went wrong
*/
public static void recursiveDeleteOnExit(Path path)throws IOException {

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

formatting issue


swaggerFileWriter.write(formattedSwaggerDef);

log.debug("Writing the swagger definition was sucessful to file" + swaggerDefFile.getAbsolutePath());

This comment has been minimized.

@harsha89

harsha89 Jun 18, 2018

Contributor

space needed after file

@eunice18

This comment has been minimized.

Copy link
Contributor Author

eunice18 commented Jun 20, 2018

hello @harsha89 I will address the comments. Currently i'm working on providing support for all the languages with improvements. I will finish this soon.

@eunice18 eunice18 force-pushed the eunice18:FM-246 branch 2 times, most recently from 73257d5 to a21f183 Jun 24, 2018

@eunice18 eunice18 force-pushed the eunice18:FM-246 branch from a21f183 to 74483de Jun 24, 2018

@eunice18

This comment has been minimized.

Copy link
Contributor Author

eunice18 commented Jun 24, 2018

hello @harsha89 I addressed the comments. I will add more languages after testing.

@harsha89

This comment has been minimized.

Copy link
Contributor

harsha89 commented Jun 29, 2018

Ok @eunice18 please include all language support in your next PRs.

@harsha89 harsha89 merged commit d4b0506 into openmrs:master Jun 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kaweesi

This comment has been minimized.

Copy link
Contributor

kaweesi commented Aug 20, 2018

this breaks down the reference apps, see: https://issues.openmrs.org/browse/FM-251

<dependency>
<groupId>io.swagger</groupId>
<artifactId>swagger-codegen</artifactId>
<version>${swaggerCondeGenVersion}</version>

This comment has been minimized.

@kaweesi

kaweesi Aug 20, 2018

Contributor

this needed to exclude javax.validation, else it was resulting into https://gist.github.com/kaweesi/82c26267cc799ce8cf183f1475099ec7 which resulted into a great bug at https://issues.openmrs.org/browse/FM-246
Fixed at: 7aad32b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.