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

Added feature for generating the parameters file #1452

Merged

Conversation

jkiddo
Copy link
Contributor

@jkiddo jkiddo commented Apr 28, 2024

@jawalonoski
Copy link
Member

Questions:

  1. Does the order of the entries in the parameters file matter? For example, loading dependencies in order: Organization,Location,Practitioner,PractitionerRole, etc. If so, then this needs alteration. If not, good to go.

  2. If you want http://localhost:8000/ to be configurable, you could add another field to synthea.properties.

Comment:

  1. This change introduces a few Checkstyle warnings:
Exporter.java:505:5: '{' at column 5 should be on the previous line. [LeftCurly]
Exporter.java:508: Line is longer than 100 characters (found 101). [LineLength]
Exporter.java:521:82: WhitespaceAround: '+' is not preceded with whitespace. [WhitespaceAround
Exporter.java:521:83: WhitespaceAround: '+' is not followed by whitespace. Empty blocks may only be represented as {} when not part of a multi-block statement (4.1.3) [WhitespaceAround]
Exporter.java:523: Line is longer than 100 characters (found 113). [LineLength]

@jkiddo
Copy link
Contributor Author

jkiddo commented Apr 29, 2024

  1. Order does not matter - at least to HAPI
  2. I'll get that fixed asap

@jkiddo
Copy link
Contributor Author

jkiddo commented Apr 30, 2024

@jawalonoski apparently the checkstyle config file in the repo is not working with the https://plugins.jetbrains.com/plugin/1065-checkstyle-idea plugin, so I'm a bit in the dark. I think I adressed all your findings.

@jkiddo jkiddo marked this pull request as ready for review April 30, 2024 18:26
@jawalonoski
Copy link
Member

Sorry for the delay. Looking at it now.

@jawalonoski jawalonoski merged commit ecc34a8 into synthetichealth:master May 16, 2024
2 checks passed
@jawalonoski
Copy link
Member

There were minor check style complaints, but I'll fix them post-merge.

Thank you for the pull request!

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.

2 participants