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

feat: add gensql.gpm.sppl-compatible setting #110

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ships
Copy link
Contributor

@ships ships commented May 2, 2024

Status

These derivations appear to work. This is "ready to merge", though some Nix refactoring could improve the code. That is not top priority before I head out for the week so merges, requests for changes, or changes by rest of team are all welcome.

Reviewing

See updates to readme. The new options are:

nix develop '.#sppl' -c clj -M:query-sppl

nix build '.#ociImgSppl'
docker load -i result
   # ---> outputs container tag
img=# tag ^

# assuming ./data/db.edn is an SPPL model
docker run -v ./data:/data -it $img gensql --db /data/db.edn

one code change

There is a minor Clojure code change and removal of the dynaload dependency which is flagged by non-code-coverage in CI. I am not worried but am curious if there is a meaningful way to validate this change's safety besides a happy-path test.

@ships ships changed the title feat: add gensql.gpm.sppl-compatible container feat: add gensql.gpm.sppl-compatible setting May 2, 2024
@ships ships mentioned this pull request May 2, 2024
@ships ships force-pushed the ships/chore/add-sppl-container branch 2 times, most recently from 139d380 to 9e5f11d Compare May 8, 2024 23:30
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.89%. Comparing base (32adb67) to head (55d0d74).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #110   +/-   ##
=======================================
  Coverage   75.89%   75.89%           
=======================================
  Files          30       30           
  Lines        1535     1535           
  Branches       64       64           
=======================================
  Hits         1165     1165           
  Misses        306      306           
  Partials       64       64           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ships ships force-pushed the ships/chore/add-sppl-container branch 2 times, most recently from bcc92b9 to f208db1 Compare May 10, 2024 00:26
@ships ships marked this pull request as ready for review May 10, 2024 00:33
@ships ships force-pushed the ships/chore/add-sppl-container branch from 1d28a42 to 4236854 Compare May 10, 2024 00:41
@ships ships requested review from Schaechtle and KingMob May 10, 2024 00:44
src/gensql/query/db.cljc Outdated Show resolved Hide resolved
@ships ships force-pushed the ships/chore/add-sppl-container branch from ab6852d to 55d0d74 Compare May 10, 2024 22:11
Copy link
Contributor

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants