Skip to content

Facette scan#17

Open
arowana755 wants to merge 1 commit intomasterfrom
facette_scan
Open

Facette scan#17
arowana755 wants to merge 1 commit intomasterfrom
facette_scan

Conversation

@arowana755
Copy link
Copy Markdown

Add a function filtering some wrong mesh generations. It doesn't filter everything, but it can catch some poorly created mesh.

Copy link
Copy Markdown
Contributor

@edgar-bonet edgar-bonet left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! I still have to take a deep look at this code, but I quickly noticed a few issues, so I am requesting some changes that you can perform while I dig more deeply into the code.

First of all, there are quite a few changes here that have nothing to do with the purpose of the pull request: const correctness, moving a function from a header to a cpp file... These should ideally me moved to another pull request. At the very least, they should be in separate commits.

Other things that deserve fixing:

  • avoid introducing compiler warnings; for instance “comparison of integer expressions of different signedness”

  • avoid introducing trailing whitespace.

  • in English, “facette” is spelled “facet”

  • in English, there is no space before a colon (write “Error: …”)

  • messages output to the terminal should end in '\n'

  • also, please rebase on top of master

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.

2 participants