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

Replace std::shared_ptr by std::unique_ptr in the heating models. #2882

Merged
merged 2 commits into from Apr 25, 2019

Conversation

bangerth
Copy link
Contributor

Strictly speaking, this is an incompatible change because the heating model manager
exports its list of heating models. I suspect that few plugins we don't have
in the tree ourselves (which I have all converted) make use of this.

Related to #2375 and #2811.

@tjhei
Copy link
Member

tjhei commented Apr 11, 2019

The following tests FAILED:
	312 - material_model_dependencies_tangurnis (Failed)
	584 - tangurnis_ba (Failed)
	585 - tangurnis_ba_custom (Failed)
	586 - tangurnis_tala (Failed)
	587 - tangurnis_tala_c (Failed)
	588 - tangurnis_tala_implicit (Failed)
``
should we delay this change until after 2.1? I am more happy to break things then.

@bangerth
Copy link
Contributor Author

Let's try this again.
/rebuild

It's ok with me if we want to postpone this until after the release. I realize that it's an incompatible change. Anyone got an opinion on a changelog entry?

@gassmoeller
Copy link
Member

Unless you have checked all benchmarks and cookbooks I would vote for postponing this after the release. Who knows what hidden plugins rely on this (ideally we should have a test for all cookbooks and benchmarks of course).

@gassmoeller
Copy link
Member

Since we branched off the release we can merge this. About the changelog entry: Do we require changelog entries that change the API, or only entries if we change the parameter files? I do not think we had many entries with interface changes, so maybe we just go ahead and merge this? Users who found and used this function can likely find it again and change the type.

@bangerth
Copy link
Contributor Author

Let me know what you prefer and I'll do it. I don't feel strongly.

@gassmoeller
Copy link
Member

Lets just merge it. I think the function is not used widely.

@gassmoeller gassmoeller merged commit 3508a7d into geodynamics:master Apr 25, 2019
@bangerth bangerth deleted the shared-2 branch April 25, 2019 03:49
@bangerth bangerth mentioned this pull request May 2, 2019
freddrichards pushed a commit to freddrichards/aspect that referenced this pull request May 20, 2019
Replace std::shared_ptr by std::unique_ptr in the heating models.
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