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.

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

Removing DEAL_II_MPI_CONST_CAST #13488

Merged
merged 1 commit into from Mar 6, 2022
Merged

Removing DEAL_II_MPI_CONST_CAST #13488

merged 1 commit into from Mar 6, 2022

Conversation

pengfej
Copy link
Contributor

@pengfej pengfej commented Mar 4, 2022 •
edited by marcfehling

Fix based on #13474

marcfehling
marcfehling approved these changes Mar 4, 2022
View reviewed changes
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.

Looks good to me!

source/distributed/tria_base.cc Outdated
@@ -1564,8 +1561,8 @@ namespace parallel
{
ierr = MPI_File_write_at(fh,
my_global_file_position,
DEAL_II_MPI_CONST_CAST(
src_data_variable.data()),

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

source/distributed/tria_base.cc Outdated
@@ -1577,8 +1574,8 @@ namespace parallel
ierr =
MPI_File_write_at(fh,
my_global_file_position,
DEAL_II_MPI_CONST_CAST(
src_data_variable.data()),

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

source/distributed/tria_base.cc
Comment on lines +1546 to 1549
ierr = MPI_Exscan(&size_on_proc,
&prefix_sum,
1,
MPI_UINT64_T,
Copy link
Member

Choose a reason for hiding this comment

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

Here and below: Interesting how clang-format handles these cases

bangerth
bangerth reviewed Mar 6, 2022
View reviewed changes
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

OK if you address @marcfehling 's two comments.

@pengfej
Copy link
Contributor Author

pengfej commented Mar 6, 2022

OK if you address @marcfehling 's two comments.

All set

@bangerth
Copy link
Member

bangerth commented Mar 6, 2022

Thanks!

Can I also ask that you squash the commits? In general, since we're going to ask for commits to be squashed, just do that whenever you make changes requested by reviewers. In other words, whenever you push a new version of a patch, make sure that the commits would be as if you had written the code in exactly that way right from the start.

@pengfej
Removing DEAL_II_MPI_CONST_CAST
b05dc25 
Fix based on dealii#13474

forgot about indent

more indent

suggested changes
@pengfej
Copy link
Contributor Author

pengfej commented Mar 6, 2022

Thanks!

Can I also ask that you squash the commits? In general, since we're going to ask for commits to be squashed, just do that whenever you make changes requested by reviewers. In other words, whenever you push a new version of a patch, make sure that the commits would be as if you had written the code in exactly that way right from the start.

Just did! I will keep that in mind. Thanks for reminding.

@bangerth bangerth merged commit f09bc52 into dealii:master Mar 6, 2022
@tjhei tjhei mentioned this pull request Mar 9, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bangerth bangerth bangerth left review comments

@tjhei tjhei tjhei approved these changes

@marcfehling marcfehling marcfehling approved these changes

Assignees
No one assigned
Labels
ready to test Reviewed and ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pengfej @bangerth @tjhei @marcfehling