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

fmt: add with_os_api option #5773

Merged
merged 7 commits into from
Jul 20, 2021
Merged

fmt: add with_os_api option #5773

merged 7 commits into from
Jul 20, 2021

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Jun 5, 2021

Specify library name and version: lib/1.0

closes #5771

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@SpaceIm SpaceIm mentioned this pull request Jun 5, 2021
Closed
@github-actions github-actions bot mentioned this pull request Jun 5, 2021
Open
@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 5, 2021
@SpaceIm SpaceIm reopened this Jun 5, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 5, 2021
@SpaceIm SpaceIm reopened this Jun 5, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 5, 2021
@SpaceIm SpaceIm reopened this Jun 5, 2021
madebr
madebr reviewed Jun 5, 2021
View reviewed changes
recipes/fmt/all/conanfile.py Outdated
"shared": [True, False],
"fPIC": [True, False],
"with_fmt_alias": [True, False],
"with_os": [True, False],
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like the with_os name. It's too close to self.settings.os
See fmtlib/fmt#1654 (comment)
How about with_os_api or something similar?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point. I second with_os_api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

madebr
madebr reviewed Jun 5, 2021
View reviewed changes
recipes/fmt/all/conanfile.py Outdated
"shared": False,
"fPIC": True,
"with_fmt_alias": False,
"with_os": True,
Copy link
Contributor

@madebr madebr Jun 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe disable the option in config_options when os not in Windows, Linux, FreeBSD, or not tools.is_apple_os?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how likely it is, but I can imagine use cases where someone may not want the OS support compiled in to the library when targeting Windows, Linux, FreeBSD, etc. One such reason is minimizing the size of the binary.

@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 6, 2021
@SpaceIm SpaceIm reopened this Jun 6, 2021
@SpaceIm SpaceIm changed the title fmt: add with_os option fmt: add with_os_api option Jun 6, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 6, 2021
@SpaceIm SpaceIm reopened this Jun 6, 2021
@SpaceIm SpaceIm closed this Jun 8, 2021
@SpaceIm SpaceIm reopened this Jun 8, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@prince-chrismc
Copy link
Contributor

Another candidate for removing older versions

@ghost ghost mentioned this pull request Jun 21, 2021
Merged
4 tasks
@conan-center-bot

This comment has been minimized.

Sign in to view
@SpaceIm SpaceIm closed this Jun 30, 2021
@SpaceIm SpaceIm reopened this Jun 30, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@ghost ghost mentioned this pull request Jul 3, 2021
Merged
4 tasks
@SpaceIm SpaceIm closed this Jul 7, 2021
@SpaceIm SpaceIm reopened this Jul 7, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@uilianries uilianries closed this Jul 16, 2021
@uilianries uilianries reopened this Jul 16, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@uilianries uilianries closed this Jul 19, 2021
@uilianries uilianries reopened this Jul 19, 2021
@conan-center-bot
Copy link
Collaborator

All green in build 17 (60b8d5cd6a6034d1f96c7806bf33bd1ff97df81f):

  • fmt/5.3.0@:
    All packages built successfully! (All logs)

  • fmt/6.0.0@:
    All packages built successfully! (All logs)

  • fmt/6.1.0@:
    All packages built successfully! (All logs)

  • fmt/6.2.0@:
    All packages built successfully! (All logs)

  • fmt/6.1.2@:
    All packages built successfully! (All logs)

  • fmt/7.0.2@:
    All packages built successfully! (All logs)

  • fmt/7.0.1@:
    All packages built successfully! (All logs)

  • fmt/6.1.1@:
    All packages built successfully! (All logs)

  • fmt/7.1.2@:
    All packages built successfully! (All logs)

  • fmt/6.2.1@:
    All packages built successfully! (All logs)

  • fmt/7.0.3@:
    All packages built successfully! (All logs)

  • fmt/7.1.1@:
    All packages built successfully! (All logs)

  • fmt/8.0.1@:
    All packages built successfully! (All logs)

  • fmt/7.1.0@:
    All packages built successfully! (All logs)

  • fmt/7.1.3@:
    All packages built successfully! (All logs)

  • fmt/8.0.0@:
    All packages built successfully! (All logs)

@prince-chrismc
Copy link
Contributor

There are versions missing from the build... do we need a rebase?

@conan-center-bot conan-center-bot merged commit 5f48b95 into conan-io:master Jul 20, 2021
@SpaceIm SpaceIm deleted the feature/fmt-with-os-option branch July 20, 2021 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwillikers jwillikers jwillikers approved these changes

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@uilianries uilianries uilianries approved these changes

@madebr madebr madebr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[package] <fmt>/<7.1.3>: Add Conan option to toggle OS support to enable use in bare metal applications
6 participants
@SpaceIm @conan-center-bot @madebr @prince-chrismc @uilianries @jwillikers