-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Synchronize porting guide with precice-v3
#298
Conversation
…de-porting-v2-3.md
@davidscn @carme-hp @MakisH @IshaanDesai @fsimonis: If you see something here that is wrong: Please directly fix it on the branch. |
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.
Looks good 👍 thanks for resolving the conflicts.
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.
All changes make sense. I checked all commits, everything seems to be there.
- Replace `precice::constants::*` with `isActionRequired()` and `markActionFulfilled()` with their respective requirement clause: `requiresInitialData()`, `requiresReadingCheckpoint()` or `requiresWritingCheckpoint()`. If these requirements are checked, then they are promised to be acted on. | ||
- Replace `isMeshConnectivityRequired` with `requiresMeshConnectivityFor`. | ||
- Replace `isGradientDataRequired` with `requiresGradientDataFor`. Instead of the input argument dataID, pass the meshName and dataName. | ||
- Replace `isActionRequired()` with their respective requirement clause: `requiresInitialData()`, `requiresReadingCheckpoint()` or `requiresWritingCheckpoint()`. | ||
- Remove `precice::constants::*` and corresponding `#include` statements as they are no longer needed. | ||
- Remove `markActionFullfiled()`. If `requiresInitialData()`, `requiresReadingCheckpoint()`, or `requiresWritingCheckpoint()` are called, then they are promised to be acted on. Therefore, `markActionFullfiled()` is no longer needed. | ||
- Replace `isMeshConnectivityRequired` with `requiresMeshConnectivityFor`. Instead of the input argument `meshID`, pass the `meshName`. | ||
- Replace `isGradientDataRequired` with `requiresGradientDataFor`. Instead of the input argument `dataID`, pass the `meshName` and `dataName`. |
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.
I like how this part is broken down to simpler, clearer steps. 👍
(whoever did that: good job! -- that's @IshaanDesai )
I just did a merge of
master
intoprecice-v3
and resolved the resulting merge conflict in the porting guide.After resolving the merge conflict, I created this branch and did
git checkout precice-v3 pages/docs/couple-your-code/couple-your-co…de-porting-v2-3.md
. The reson for this PR is to be able to review my resolution of the merge conflict.To avoid this in the future: Generally, we should always keep the porting guide on
master
andprecice-v3
synchronized, because there is no reason to not directly publish changes in the porting guide to master. The purpose ofprecice-v3
is to develop the documentation that we cannot publish yet, because it would overwrite the v2 documentation that is still relevant for users.There are two important rules:
master
. Don't forget to mergemaster
intoprecice-v3
(e.g. Porting guide hint for removed preallocation option #288 and Clarify some instructions in porting guide #287).precice-v3
, you should make sure to synchronize the porting guide withmaster
.Relevant changes to review:
@davidscn #290
@carme-hp #287
@IshaanDesai 91efb0a
@MakisH 778ce08
@fsimonis c8740c3