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

cautious treatment of compiler in b2 generator #4202

Merged

Conversation

grisumbras
Copy link
Contributor

@grisumbras grisumbras commented Dec 26, 2018

Using settings.get_safe more actively. Conversion of Nones to strings.
Fixes #4201

Changelog: Bugfix: b2 generator was failing when package recipe didn't use compiler setting
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.

Using settings.get_safe more actively. Conversion of Nones to strings.
Fixes conan-io#4201
@CLAassistant
Copy link

CLAassistant commented Dec 26, 2018

CLA assistant check
All committers have signed the CLA.

@jgsogo jgsogo requested a review from danimtb December 27, 2018 10:08
Copy link
Member

@danimtb danimtb 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 simplification! We are willing to review the whole b2 generator so this is very welcome 😄

@danimtb
Copy link
Member

danimtb commented Dec 27, 2018

Hi @grisumbras!

Thanks for your PR. Could you please include a brief changelog in the description of this PR? This will help us to include this in the next release. Thanks! 😃

@danimtb
Copy link
Member

danimtb commented Dec 27, 2018

Oh, I forgot to mention! It would be nice to write a test with the use case that was failing before this changes. I could help with it in case you are not familiar with them or don't have time. Just ping me!

@danimtb danimtb self-assigned this Dec 27, 2018
@danimtb danimtb added this to the 1.12 milestone Dec 27, 2018
@grisumbras
Copy link
Contributor Author

Should I prepend a commit with test or just add a commit?

@danimtb
Copy link
Member

danimtb commented Dec 27, 2018

An additional commit will be fine. We mostly review the diff so it doesn't matter that much

@grisumbras
Copy link
Contributor Author

Also, how should I create a ConanFile instance suitable for constructing a generator?

@danimtb
Copy link
Member

danimtb commented Dec 27, 2018

Have a look at https://github.com/conan-io/conan/blob/develop/conans/test/unittests/client/generators/b2_test.py

I guess your test should be very similar

@grisumbras
Copy link
Contributor Author

I updated the PR description and added a test.

Copy link
Member

@danimtb danimtb left a comment

Choose a reason for hiding this comment

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

Cool! Thanks @grisumbras

@danimtb danimtb merged commit db11550 into conan-io:develop Dec 27, 2018
NoWiseMan pushed a commit to NoWiseMan/conan that referenced this pull request Jan 9, 2019
* cautious treatment of compiler in b2 generator

Using settings.get_safe more actively. Conversion of Nones to strings.
Fixes conan-io#4201

* test for b2 generator support of empty settings
@lasote
Copy link
Contributor

lasote commented Jan 17, 2019

@danimtb please add the changelog and docs entries to this PR body.

@lasote
Copy link
Contributor

lasote commented Jan 17, 2019

My bad, it was already there.

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.

b2 generator fails to work when some settings aren't used by the project
4 participants