-
Notifications
You must be signed in to change notification settings - Fork 16
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
accept flag #478
accept flag #478
Conversation
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.
LGTM! Except for very small things
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.
lgtm!
conjure_oxide/tests/integration/basic/bool/01/bool-01.expected-minion.solutions 2.json
Outdated
Show resolved
Hide resolved
conjure_oxide/tests/integration/basic/bool/01/bool-01.expected-parse.serialised 2.json
Outdated
Show resolved
Hide resolved
conjure_oxide/tests/integration/basic/bool/01/bool-01.expected-rewrite.serialised 2.json
Outdated
Show resolved
Hide resolved
conjure_oxide/tests/integration/basic/bool/03/bool-03.expected-minion.solutions 2.json
Outdated
Show resolved
Hide resolved
conjure_oxide/tests/integration/basic/bool/03/bool-03.expected-parse.serialised 2.json
Outdated
Show resolved
Hide resolved
conjure_oxide/tests/integration/basic/bool/03/bool-03.expected-rewrite.serialised 2.json
Outdated
Show resolved
Hide resolved
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.
@ozgurakgun ready to merge?
@a-nosnitram @Soph1514 FYI: I have added a commit to this PR to remove some duplicate files (.expected-trace 2) |
Code and Documentation Coverage ReportDocumentation CoverageClick to view documentation coverage for this PR
Click to view documentation coverage for main
Code Coverage SummaryThis PR: Detailed Report
Main: Detailed Report
Coverage Main & PR Coverage ChangeLines coverage changed by 0.00% and covered lines changed by 3
Functions coverage changed by 0.00% and covered lines changed by 0
Branches... coverage: No comparison data available |
Thanks @niklasdewally, @a-nosnitram I was thinking we could write code to copy all generated files to expected instead of doing them one by one. Thoughts? |
Sounds good. Probably would be better in a follow-up PR, as I need this PR merged to get my tests passing. |
ok, can you capture this please @a-nosnitram and @Soph1514? |
Makes sense, will look into that soon! |
making accept work on tracing