-
Notifications
You must be signed in to change notification settings - Fork 2
Implement Tier 1 Transpiler #20
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
Conversation
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
tokebe
left a comment
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.
Looks pretty good-- I've made a few notes, and you've got a few complaints from the github actions (sorry for not having those set up right prior).
You'll want to make sure to commit the lockfile when adding dependencies. Also, to shortcut waiting on actions to see if you've upset the type checker/etc., I'd recommend running task fixup in the workspace to see what the linter/type checker/etc. have to say. You can also run task hook to make it so these run automatically and must pass when making a commit.
Codecov is complaining, as well. For the sake of time, don't worry about writing tests right now, but if you have time in the future please do add tests if you can.
|
All suggested code changes were applied. Test coverage will definitely be addressed in future iterations. |
|
The PR still has problems-- linting and typechecking have pointed out errors that would cause runtime failures (request_timeout vs query_timeout in the config, etc.) For the sake of time, I've pulled your PR into a branch and am fixing it, but in the future you'll want to make sure you're using |
Add
Tier 1transpiler logic and associated driver class for generating Elasticsearch query payloads for given TRAPI queries.