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

Feature/447 get versioned ehr status #415

Merged
merged 23 commits into from
Feb 16, 2021

Conversation

jakesmolka
Copy link
Contributor

@jakesmolka jakesmolka commented Jan 26, 2021

Changes

Related issue

closes https://github.com/ehrbase/project_management/issues/447

Additional information and checks

  • Add and fix license headers
  • Pull request linked in changelog

@jakesmolka
Copy link
Contributor Author

@birgerhaarbrandt Who should review the integration test definition (see wiki link)? I just realized we don't have to wait with reviewing until the CI is fixed.

@jakesmolka jakesmolka marked this pull request as ready for review February 4, 2021 11:27
@jakesmolka jakesmolka marked this pull request as draft February 4, 2021 11:31
@wlad wlad self-requested a review February 4, 2021 22:05
@wlad wlad marked this pull request as ready for review February 4, 2021 22:05
Copy link
Contributor

@wlad wlad left a comment

Choose a reason for hiding this comment

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

looks good 👍

  • previously failing test now works locally and on CI

I committed some changes

  • enhanced tests a little bit (cleaned up, removed some unnecessary lines)
  • added a few more tests
  • added bug trace to issue ehrbase/project_management/issues/458 (and marked related test as not-ready)
  • fixed sometime failing EHRbase startup in CI script

@jakesmolka
Copy link
Contributor Author

jakesmolka commented Feb 8, 2021

@jakesmolka Check if the timezone converting in the integration tests will break if EHR timestamp gets correct format in the future.

Edit: Timestamp handling in C.6 C) 3. looks alright. And the EHR timestamp response isn't used anymore, so there will be no problem with a change of format. 👍

@ppazos
Copy link
Contributor

ppazos commented Feb 9, 2021

@jakesmolka I have added my comments about the documentation here https://wiki.vitagroup.ag/pages/viewpage.action?spaceKey=ETHERCIS&title=Versioned+EHR_STATUS

@sonarcloud
Copy link

sonarcloud bot commented Feb 16, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

71.4% 71.4% Coverage
0.0% 0.0% Duplication

@jakesmolka jakesmolka merged commit 0697195 into develop Feb 16, 2021
@jakesmolka jakesmolka deleted the feature/447_get_versioned_ehr_status branch February 16, 2021 15:14
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.

None yet

3 participants