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

Fix for warnings from world builder. #5230

Merged
merged 1 commit into from Jul 9, 2023

Conversation

MFraters
Copy link
Member

@MFraters MFraters commented Jul 9, 2023

@bobmyhill and @anne-glerum could you test if this fixes the issue?

@tjhei It would be great to take this over in the release branch

@tjhei
Copy link
Member

tjhei commented Jul 9, 2023

This makes sense. Are these the warnings I reported to you?
I am surprised you want to fix things directly. Why not make a point release of WB and import the new version here? Or is that too much effort?

@MFraters
Copy link
Member Author

MFraters commented Jul 9, 2023

When and where did you send them? These where warnings reported by @anne-glerum and @bobmyhill.

I also made a pull request for the world builder in GeodynamicWorldBuilder/WorldBuilder#503. I could make a point release, I am not sure if it is worth it.

Comment on lines +307 to 308
(void) this->limit_debug_consistency_checks;
WBAssert(!this->limit_debug_consistency_checks || this->parameters.coordinate_system->natural_coordinate_system() == cartesian
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really warn about this variable being unused? It's a member variable, how does the compiler know that the function isn't used anywhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

In debug mode is isn't used anywhere. I never got this warning, but it does appear on some mac's apparently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fascinating.

@bangerth
Copy link
Contributor

bangerth commented Jul 9, 2023

I'm ok to merge as-is, in the same way as we often apply small patches to bundled packages to avoid warnings. But I'll defer to @tjhei if he has a different opinion.

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

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

Fine with me either way.

@tjhei
Copy link
Member

tjhei commented Jul 9, 2023

Here is the issue I opened: GeodynamicWorldBuilder/WorldBuilder#502

@tjhei tjhei merged commit 14bffec into geodynamics:main Jul 9, 2023
6 checks passed
@tjhei tjhei mentioned this pull request Jul 9, 2023
@MFraters
Copy link
Member Author

MFraters commented Jul 9, 2023

Here is the issue I opened: GeodynamicWorldBuilder/WorldBuilder#502

Ah, sorry. I had completely missed that. Can you confirm that it is fixed now?

@MFraters
Copy link
Member Author

MFraters commented Jul 9, 2023

@bobmyhill confirmed that the warnings are gone for him.

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.

None yet

3 participants