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

Add dd on merge #898

Merged
merged 8 commits into from
Aug 19, 2023
Merged

Add dd on merge #898

merged 8 commits into from
Aug 19, 2023

Conversation

gonuke
Copy link
Member

@gonuke gonuke commented Aug 17, 2023

Description

Add a test with Double Down on merge.

Motivation and Context

We discovered after a recent PR was merged that it broke compatibility with Double Down. For the time being, we have decided to only test that compatibility at merge and not with each PR.

Changes

Adds a matrix option to build DAGMC using Double Down. Also allows jobs to continue on error so that all options are tested even if one fails, since this is advisory post merge.

Also took the opportunity to collapse the docker builds in this workflow, and to exclude changes to this file from triggering other build/test actions.

Fixes #897

@gonuke
Copy link
Member Author

gonuke commented Aug 17, 2023

A successful demonstration of this test is here, including the currently failing Double Down test.

@gonuke gonuke requested review from pshriwise and shimwell August 17, 2023 18:35
Copy link
Member

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

This looks good to me @gonuke. Thanks for including DD in the test suite! I'm assuming that the removed block for tagging and uploading Docker images is taken care of elsewhere as a result of previous PRs.

@gonuke
Copy link
Member Author

gonuke commented Aug 18, 2023

This looks good to me @gonuke. Thanks for including DD in the test suite! I'm assuming that the removed block for tagging and uploading Docker images is taken care of elsewhere as a result of previous PRs.

It's not clear whether we want to tag/push these docker images since their purpose was just to test against upstream dependencies.

@pshriwise
Copy link
Member

Makes sense. Thanks for clarifying!

@pshriwise pshriwise merged commit 3ffee08 into svalinn:develop Aug 19, 2023
@gonuke gonuke deleted the add_dd_on_merge branch August 21, 2023 00:10
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.

test dagmc compile with double down in Docker file CI
2 participants