Updated tests, add CI hook #21
Conversation
dogversioning
force-pushed
the
mg/test_updates
branch
6 times, most recently
from
6c7bc13 to
c2a5666
Compare
dogversioning
force-pushed
the
mg/test_updates
branch
from
c2a5666 to
fcdc61f
Compare
.github/workflows/ci.yaml
Outdated
| jobs: | ||
| unittest: | ||
| name: unit tests | ||
| runs-on: ubuntu-22.04 |
There was a problem hiding this comment.
nit: maybe use latest here, since you're not doing anything specific like "apt install"ing.
.github/workflows/ci.yaml
Outdated
| - name: Get library from main | ||
| run: | | ||
| git clone https://github.com/smart-on-fhir/cumulus-library.git | ||
| cd cumulus-library | ||
| pip install -e . | ||
| cd .. |
There was a problem hiding this comment.
nit: since you don't actually end up using the checkout, you could get away with this:
- name: Get library from main
run: pip install git+https://github.com/smart-on-fhir/cumulus-library.git
Do you maybe want this after the "install dependencies" stanza? 1) you get the latest pip first that way and 2) what if your pyproject has a dependency pin like "cumulus-library >=2, <3" -- it could silently undo the install-from-main here. (I know this PR also unpins the library a bit, but still - it's the silent nature of it here that could hide issues with the ordering)
There was a problem hiding this comment.
oooh fancy, i like it.
I don't want this in the dependencies stanza, since this is currently pinned to 3.0, which is not yet released.
There was a problem hiding this comment.
when it :is: released i'm going to remove this stanza entirely.
| b_join_col = b_join_col or 'b.rxcui' | ||
| b_table = b_table or f'opioid__{steward}_vsac', | ||
| b_join_col = b_join_col or 'b.code' | ||
| b_table = b_table or f'opioid__{steward}_vsac' |
There was a problem hiding this comment.
Python is wrong here 😄 - it should really require parens or something to make the theoretical "I want a tuple" clearer
There was a problem hiding this comment.
this has bitten me in both directions more times than i'd care to count
This PR makes the following changes: