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

Fixes for MS Visual Studio 2013 #1526

Closed
wants to merge 6 commits into from
Closed

Fixes for MS Visual Studio 2013 #1526

wants to merge 6 commits into from

Conversation

magro11
Copy link

@magro11 magro11 commented Feb 7, 2016

Dear PCL development team,

this pull request contains three fixes for a proper compilation under MS Visual Studio 2013 if the surface_nurbs project is enabled. It would be great if you could merge this.

Thanks.
Marc

@@ -184,7 +184,8 @@ int main (int argc, char *argv[])
range_cond->addComparison (pcl::FieldComparison<PointOutT>::ConstPtr (new
pcl::FieldComparison<PointOutT> ("curvature", pcl::ComparisonOps::GT, threshold)));
// build the filter
pcl::ConditionalRemoval<PointOutT> condrem (range_cond);
pcl::ConditionalRemoval<PointOutT> condrem;
condrem.setCondition(range_cond);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please follow the style guide?

There is an extra space at the beginning of these two lines.
There should be a space before the opening parenthesis.

@VictorLamoine
Copy link
Contributor

Please fix style accordingly to my comment and squash your commits, it will be ok to merge after that!
Thanks for the contribution.

@magro11
Copy link
Author

magro11 commented Feb 8, 2016

The misalignment of the first level indentation were inserted since the original file contained tabs in the first level. I replaced now every tab by two spaces each. Hope that this version is now OK.

@VictorLamoine
Copy link
Contributor

No 😄
You modified indentations from code you did not modify, we don't want that.

It seems you did no other modifications than indentations in examples/features/example_difference_of_normals.cpp; please do not include this file in the pull request. (if you have done modifications, please include them but don't change indentation).

examples/segmentation/example_cpc_segmentation.cpp is ok 👍

@magro11
Copy link
Author

magro11 commented Feb 8, 2016

Ok, but then I have to adapt my identation level to the wrong style of the file since the file contained tabs as indentation. You wanted me to add two spaces as first identation in the lines I changed (in total two lines). But if I add two spaces and the rest of the file are tabs, this is also not right. Should I place tabs now in order to be consistent to the wrong style of the file or how do you want me to align my changed lines? (All this only belongs example_difference_of_normals.cpp only)

@VictorLamoine
Copy link
Contributor

@taketwo Should we fix the code style in a separate commit?

@jspricke
Copy link
Member

jspricke commented Feb 8, 2016

Yes, fix the style in one commit and put your changes in a new one.

@magro11
Copy link
Author

magro11 commented Feb 15, 2016

Are my changes now correct or is there still something to do from my side?

@jspricke
Copy link
Member

Can you please squash the commits, to review them?

@jspricke
Copy link
Member

I've fixed the commits and pushed them into master. Thanks for the fixes!

@jspricke jspricke closed this Feb 15, 2016
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.

3 participants