Skip to content

Chc conditions #1612

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
Merged
merged 19 commits into from
Oct 24, 2019
Merged

Chc conditions #1612

merged 19 commits into from
Oct 24, 2019

Conversation

noerog
Copy link
Contributor

@noerog noerog commented Oct 10, 2019

No description provided.

@noerog noerog requested a review from a team October 10, 2019 19:48
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 10, 2019
lesv
lesv approved these changes Oct 10, 2019
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

I see you've included a client library, but you are invoking REST directly -- is that really the best way to use this API? If so, LGTM, if not, maybe some conversations should be had to better use the client library.

@noerog
Copy link
Contributor Author

noerog commented Oct 11, 2019

We've had a bunch of conversations internally about this and this is the best workaround we have at the moment.

@noerog
Copy link
Contributor Author

noerog commented Oct 21, 2019

Hi lesv@, the client library actually can support this. Just updated and re-committed.

@noerog
Copy link
Contributor Author

noerog commented Oct 23, 2019

Hi lesv@, is there anything I need to do to get this merged?

@averikitsch averikitsch added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2019
@lesv
Copy link
Contributor

lesv commented Oct 23, 2019

Get it to pass Java 8

lesv
lesv approved these changes Oct 23, 2019
View reviewed changes
Copy link
Contributor

@lesv lesv left a comment

Choose a reason for hiding this comment

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

@dpebot merge when green

@noerog
Copy link
Contributor Author

noerog commented Oct 24, 2019

Is dpe@ bot supposed to have merged this?

@lesv
Copy link
Contributor

lesv commented Oct 24, 2019

It should have - one more try
@dpebot merge when green

@lesv lesv added the automerge Merge the pull request once unit tests and other checks pass. label Oct 24, 2019
@lesv
Copy link
Contributor

lesv commented Oct 24, 2019

But DPEbot doesn't appear to be listening @kurtisvg FYI
I'll try to remember to do it when the tests pass.

@kurtisvg kurtisvg merged commit fbc1ae2 into GoogleCloudPlatform:master Oct 24, 2019
@lesv
Copy link
Contributor

lesv commented Oct 24, 2019

Sort of like DPEBot. 👍

@noerog noerog deleted the chc-conditions branch November 13, 2019 21:43
bradmiro pushed a commit that referenced this pull request Dec 2, 2019
@noerog @bradmiro
Add Healthcare API snippets. (#1612)
d23f4e4 
* Add Healthcare API DICOMweb search studies by tag

* Use client library.

* Fix FhirStoreTests failures

* Remove unnecessary .dcm file and variables.

* Fix DicomStoreTests failures

* Make bucket name in tests an environment variable.

* Make bucket name in tests an environment variable.

* Fix lint, make Storage bucket hard-coded again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lesv lesv lesv approved these changes

Assignees

@lesv lesv

Labels
automerge Merge the pull request once unit tests and other checks pass. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@noerog @lesv @googlebot @averikitsch @kokoro-team @kurtisvg