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.

Sign up for GitHub

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

Jump to bottom

No schema files #159 #371

Merged
merged 7 commits into from
Nov 7, 2019
Merged

No schema files #159 #371

merged 7 commits into from
Nov 7, 2019

Conversation

prb112
Copy link
Contributor

@prb112 prb112 commented Nov 6, 2019

Signed-off-by: Paul Bastide pbastide@us.ibm.com

@prb112
No schema files #159
7176d30 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 requested a review from lmsurpre November 6, 2019 18:46
@prb112 prb112 self-assigned this Nov 6, 2019
@prb112 prb112 mentioned this pull request Nov 6, 2019
Closed
@prb112 prb112 added this to the Sprint 4 milestone Nov 6, 2019
@prb112 prb112 added automation automation documentation Improvements or additions to documentation labels Nov 6, 2019
lmsurpre
lmsurpre reviewed Nov 6, 2019
View reviewed changes
fhir-persistence-schema/src/main/java/com/ibm/fhir/schema/app/SimpleSchemaPrinter.java Outdated
Comment on lines 262 to 263
* The following classes are 'dummy implementations' that enable printing of the
* SQL.
Copy link
Member

Choose a reason for hiding this comment

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

why do we need the dummy stubs? no way to just use the existing Print classes?
if its really needed, maybe we introduce test dependency for creating the stubs from the Interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

existing print classes miss many commands and do not fully include all the commands

Copy link
Member

Choose a reason for hiding this comment

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

can we fix those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really. It'll be a pain to re-engineer to support it; I looked at it. This is the reliable way - regardless of IDatabaseTarget and IDatabaseTranslator.

@lmsurpre
Copy link
Member

lmsurpre commented Nov 6, 2019

do we really want to commit to keeping the DDL up-to-date in our docs?

@prb112
Copy link
Contributor Author

prb112 commented Nov 6, 2019 •
edited
Loading

do we really want to commit to keeping the DDL up-to-date in our docs?

Perfectly valid question... maybe teach questions/decision?
In general, I think so.

@prb112
adding comments per issue #159
c405d5c 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
prb112 and others added 3 commits November 7, 2019 09:17
@prb112 @lmsurpre
Update fhir-persistence-schema/README.md
e50eb96 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

Co-Authored-By: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112 @lmsurpre
Update fhir-persistence-schema/README.md
a50cba7 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

Co-Authored-By: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112 @lmsurpre
Update fhir-persistence-schema/README.md
a90e3a4 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

Co-Authored-By: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112
Copy link
Contributor Author

prb112 commented Nov 7, 2019

got it

@prb112
Merge branch 'master' into issue-159
49b8e96
@prb112
fix for schema output #159
9cd40f3 
Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@prb112 prb112 mentioned this pull request Nov 7, 2019
Open
lmsurpre
lmsurpre approved these changes Nov 7, 2019
View reviewed changes
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

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

LGTM

@prb112 prb112 merged commit c7d4115 into master Nov 7, 2019
@prb112 prb112 deleted the issue-159 branch November 7, 2019 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmsurpre lmsurpre lmsurpre approved these changes

Assignees

@prb112 prb112

Labels
automation automation documentation Improvements or additions to documentation
Projects
None yet
Milestone
Sprint 4
Development

Successfully merging this pull request may close these issues.
2 participants
@prb112 @lmsurpre