-
Notifications
You must be signed in to change notification settings - Fork 213
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
Use relative paths for access to modules (avoid sys.path fiddling) #151
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
b72c65e
to
c680f40
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test just written to bump up our code coverage from 35% to 45%
These are unnecessary nowadays and dropping them will reduce complexity and improve coverage reporting.
c680f40
to
54e6313
Compare
if: github.ref != 'refs/heads/main' | ||
# Disable coverage checking from forks until there is a secure fix for: | ||
# https://github.com/orgoro/coverage/issues/259 | ||
if: github.ref != 'refs/heads/main' && ${{ ! github.event.pull_request.head.repo.fork }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated, but I noticed this problem when trying to to land #109 -- coverage fails on fork branches 😦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh fascinating - i guess that should probably be something we do elsewhere.
if: github.ref != 'refs/heads/main' | ||
# Disable coverage checking from forks until there is a secure fix for: | ||
# https://github.com/orgoro/coverage/issues/259 | ||
if: github.ref != 'refs/heads/main' && ${{ ! github.event.pull_request.head.repo.fork }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh fascinating - i guess that should probably be something we do elsewhere.
This is a cherry-pick from #134 (which has other unrelated patches in it, which is why I'm making a separate, focused PR) and a commit of my own invention to drop an old python2-workaround from the templates (which gets us back in sync with how fhir-parser does it in the sample templates).
This drops python2 support without bumping the major version. I think though, that fhirclient's versioning is a little unique. Maybe it's worth talking about. Here's how I was thinking about it:
pip install
run that is stuck on old runtimes won't download your new package)If we don't like the above approach, I could buy disconnecting the major version from the FHIR version and just making it clear what the support matrix is (like our README kind of already does).
Fixes #110