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

Update minimum Boost version required #509

Merged
merged 1 commit into from
Sep 6, 2020
Merged

Update minimum Boost version required #509

merged 1 commit into from
Sep 6, 2020

Conversation

varunagrawal
Copy link
Collaborator

This PR fixes #508.

@varunagrawal varunagrawal added the feature New proposed feature label Sep 3, 2020
@varunagrawal varunagrawal self-assigned this Sep 3, 2020
@ProfFan
Copy link
Collaborator

ProfFan commented Sep 3, 2020

I would prefer a warning instead of hard fail (if GTSAM still builds against 1.43?)

@varunagrawal
Copy link
Collaborator Author

I haven't tried building against 1.43 but from my experience with the CI, I don't think it will.

@varunagrawal
Copy link
Collaborator Author

@ProfFan my main concern is that if we just give a warning (rather than failing), some users may not see the warning as important and come and raise issues here. By providing a hard fail, we emphasize their Boost version is not supported.

Thoughts @dellaert?

@dellaert
Copy link
Member

dellaert commented Sep 6, 2020

@ProfFan my main concern is that if we just give a warning (rather than failing), some users may not see the warning as important and come and raise issues here. By providing a hard fail, we emphasize their Boost version is not supported.

Thoughts @dellaert?

Agreed. We should put minimum version in cmake which is the true minimal version for a bug-free GTSAM. I think cmake should hard-error if not found.

ProfFan
ProfFan approved these changes Sep 6, 2020
View reviewed changes
Copy link
Collaborator

@ProfFan ProfFan left a comment

Choose a reason for hiding this comment

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

Sure, @dellaert agreed :)

@varunagrawal varunagrawal merged commit 8b1516f into develop Sep 6, 2020
@varunagrawal varunagrawal deleted the feature/boost-version-bump branch September 6, 2020 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ProfFan ProfFan ProfFan approved these changes

@chrisbeall chrisbeall Awaiting requested review from chrisbeall

Assignees

@varunagrawal varunagrawal

Labels
feature New proposed feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bump minimum Boost version
3 participants
@varunagrawal @ProfFan @dellaert