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

Mesh_3 - Mesh_criteria_3 cleaning #7844

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

janetournois
Copy link
Member

@janetournois janetournois commented Nov 2, 2023

Summary of Changes

As spotted in PR #7532, there were named parameters available in the constructor of Mesh_criteria_3 that had never been documented or used.

This commit, written before the publication of Mesh_3 with features in CGAL-3.8, removed the documentation of these criteria before their publication.

Release Management

  • Affected package(s): Mesh_3
  • License and copyright ownership: unchanged

@janetournois
Copy link
Member Author

@lrineau I also added the use of get_parameter_reference() for all the parameters that may be a MeshDomainField_3 (distance and size)

@sloriot sloriot added the Batch_2 Second Batch of PRs under testing label Nov 9, 2023
@sloriot sloriot added Under Testing and removed Batch_2 Second Batch of PRs under testing labels Nov 21, 2023
@sloriot
Copy link
Member

sloriot commented Nov 22, 2023

Most probably responsible for errors in (periodic) Mesh_3 test/ex in CGAL-6.0-Ic-112

@sloriot sloriot added Tests failing Batch_1 First Batch of PRs under testing and removed Under Testing labels Nov 22, 2023
@janetournois
Copy link
Member Author

My last commit 7fdc36d highlights the fact that it was possible to use facet_size together with facet_sizing_field and sizing_field, which was imho confusing, but could still be considered as a feature.
@lrineau do you still agree that we should keep only facet_size (and other dimensions counterparts)?

@lrineau
Copy link
Member

lrineau commented Nov 24, 2023

My last commit 7fdc36d highlights the fact that it was possible to use facet_size together with facet_sizing_field and sizing_field, which was imho confusing, but could still be considered as a feature. @lrineau do you still agree that we should keep only facet_size (and other dimensions counterparts)?

Yes I agree.

@sloriot sloriot added Under Testing and removed Tests failing Batch_1 First Batch of PRs under testing labels Nov 28, 2023
@afabri
Copy link
Member

afabri commented Nov 30, 2023

Maybe this PR is responsible for this warning:

/Users/sloriot/CGAL/testsuite/CGAL-6.0-Ic-118/cmake/platforms/x86-64_Darwin-18.5_Apple-clang-default_Release/test/Mesh_3/test_mesh_criteria_creation.cpp:96:6: warning: unused variable 'facet_size_nok' [-Wunused-variable]
  FT facet_size_nok = radius_facet/FT(10);
     ^
/Users/sloriot/CGAL/testsuite/CGAL-6.0-Ic-118/cmake/platforms/x86-64_Darwin-18.5_Apple-clang-default_Release/test/Mesh_3/test_mesh_criteria_creation.cpp:137:6: warning: unused variable 'cell_size_nok' [-Wunused-variable]
  FT cell_size_nok = radius_cell/FT(10);
     ^

now there is only one possible sizing definition left, they have
become useless
@sloriot
Copy link
Member

sloriot commented Dec 7, 2023

Successfully tested in CGAL-6.0-Ic-122

@lrineau lrineau self-assigned this Dec 7, 2023
@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Dec 7, 2023
@lrineau lrineau merged commit ef881a1 into CGAL:master Dec 11, 2023
9 checks passed
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Dec 11, 2023
@lrineau lrineau deleted the Mesh_3-remove_unused_criteria-GF branch December 11, 2023 13:12
ange-clement added a commit to ange-clement/cgal that referenced this pull request Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants