8000 Add isMeshConnectivityRequired() by erikscheurer · Pull Request #31 · precice/matlab-bindings · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add isMeshConnectivityRequired() #31

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 15, 2021
Merged

Add isMeshConnectivityRequired() #31

merged 3 commits into from
Sep 15, 2021

Conversation

erikscheurer
Copy link
Contributor

Added Function isMeshConnectivityRequired which is implemented in the preCICE interface but not in the matlab bindings.

IshaanDesai
IshaanDesai approved these changes Sep 14, 2021
View reviewed changes
Copy link
Member
@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

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

At the first glance the changes look good. @BenjaminRodenberg is there any way we can do testing without a lot of effort? Or do you think for now its okay to just merge newer API functions?

BenjaminRodenberg
BenjaminRodenberg approved these changes Sep 14, 2021
View reviewed changes
Copy link
Member
@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

I also don't see any obvious problems. For testing there is unfortunately no real procedure in place. You could try to call the new API function in the solverdummy as a small test. This is already better than nothing.

@IshaanDesai
Copy link
Member
IshaanDesai commented Sep 15, 2021 •
edited
Loading

Ready to merge, @erikscheurer in the comment above the function can you add a EXPERIMENTAL keyword so that a potential user clearly sees that this API function is experimental and untested. After some discussion it is understood that the function isMeshConnectivityRequired() is a regular feature and not experimental

@IshaanDesai IshaanDesai merged commit 348ea8d into develop Sep 15, 2021
@IshaanDesai IshaanDesai deleted the add-isMeshConnectivityRequired branch October 7, 2021 13:15
@IshaanDesai IshaanDesai mentioned this pull request Oct 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminRodenberg BenjaminRodenberg BenjaminRodenberg approved these changes

@IshaanDesai IshaanDesai IshaanDesai approved these changes

Assignees

@erikscheurer erikscheurer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erikscheurer @IshaanDesai @BenjaminRodenberg
0