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.

Sign up for GitHub

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

Jump to bottom

Improve ASCIIReader::setInputFields and deprecate old version #1690

Merged
merged 1 commit into from
Sep 23, 2016
Merged

Improve ASCIIReader::setInputFields and deprecate old version #1690

merged 1 commit into from
Sep 23, 2016

Conversation

SergioRAgostinho
Copy link
Member

@SergioRAgostinho SergioRAgostinho commented Aug 23, 2016 •
edited
Loading

Addresses the second issue in #1688.

We need to decide if we commit this to 1.8.1 or not, since it prevents compilation of the tests in MSVC. Closes #1688.

@SergioRAgostinho SergioRAgostinho added status: needs decision needs: code review Specify why not closed/merged yet labels Aug 23, 2016
@taketwo
Copy link
Member

taketwo commented Aug 23, 2016

LGTM
I don't see why not including this into 1.8.1, it does not break anything.

@taketwo taketwo added status: ready to merge and removed needs: code review Specify why not closed/merged yet labels Aug 23, 2016
@SergioRAgostinho
Copy link
Member Author

Hmm. You're right. It preserves the old symbol and it's not modifying the size of ASCIIReader, it just adds a new symbol.

@SergioRAgostinho SergioRAgostinho added this to the pcl-1.8.1 milestone Aug 23, 2016
@taketwo
Copy link
Member

taketwo commented Aug 23, 2016

Also all existing code that uses this function should remain unambiguous.

@SergioRAgostinho
Copy link
Member Author

I searched for all the occurrences of setInputFields throughout the project and only found the one I modified.

@UnaNancyOwen
Copy link
Member

I was confirm that test_io can build successfully on MSVC.
LGTM

@UnaNancyOwen UnaNancyOwen mentioned this pull request Sep 13, 2016
Closed
@jspricke jspricke merged commit 2235fd3 into PointCloudLibrary:master Sep 23, 2016
@SergioRAgostinho SergioRAgostinho deleted the ascii-set-input-fields branch December 21, 2016 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
pcl-1.8.1
Development

Successfully merging this pull request may close these issues.
4 participants
@SergioRAgostinho @taketwo @UnaNancyOwen @jspricke