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

Clean up precice-ci.yml #93

Merged
merged 4 commits into from Apr 14, 2023
Merged

Clean up precice-ci.yml #93

merged 4 commits into from Apr 14, 2023

Conversation

peterrum
Copy link
Member

@peterrum peterrum commented Jan 4, 2022

FYI @davidscn

@bangerth
Copy link
Member

I don't know anything about how these CI files should be written. This requires someone else's approval.

Copy link
Member

@marcfehling marcfehling 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 really nice when you get the docker image in the github-actions script.

.github/workflows/precice-ci.yml Outdated Show resolved Hide resolved
.github/workflows/precice-ci.yml Outdated Show resolved Hide resolved
.github/workflows/precice-ci.yml Outdated Show resolved Hide resolved
.github/workflows/precice-ci.yml Outdated Show resolved Hide resolved
@bangerth
Copy link
Member

@peterrum What's the status here?

@marcfehling
Copy link
Member

marcfehling commented Apr 8, 2023

I've solved the merge conflict by rebasing. @peterrum -- I hijacked your PR 🏴‍☠️

The permission denied error can be fixed by overriding the default container user, see also actions/checkout#956. I will push a fix in a separate commit.

I will also apply my suggestions in a separate commit. Updating to the latest preCICE 2.5.0 release yields a different output and thus lets the test fail. I will thus update to preCISE 2.4.0 instead. We can consider an update to 2.5.0 in a separate PR.

Copy link
Member Author

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

@marcfehling Thanks for the follow-up commits! Looks good to me. I cannot approve my own PR^^

@marcfehling
Copy link
Member

The PR has been around for a year. Let's merge this.

@marcfehling marcfehling merged commit a6e111e into dealii:master Apr 14, 2023
2 checks passed
@marcfehling marcfehling added the CI label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants