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

create iso_week header only library in cmake-file #503

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

allspark
Copy link

@allspark allspark commented Nov 7, 2019

No description provided.

@lrusak
Copy link

lrusak commented Sep 9, 2020

any updates here? it would be nice to have the iso_week.h header installed via cmake

@mellery451
Copy link
Contributor

I think the cmake files have diverged a bit and this PR would need to be rebased/updated if it were to get merged. That said, there are a few questionable changes (Threads ONLY for MSVC seems odd, for example...) that I think would need review before including. Also, I wonder if instead of JUST iso_week, there should be a "date extras" interface library that includes all the other interesting headers that aren't part of core date. We'd probably need @HowardHinnant to advise, but from a simplicity point of view, it seems like bundling all the optional/additional headers into an "extras" target would be simplest. The only downside is if one or more of these headers brings in additional dependencies - that just complicates things. We might also need this extra target to be optional in case some packagers don't want to include it.

@HowardHinnant
Copy link
Owner

A "date extras" interface that includes everything sounds good to me. All of the extra headers are header-only libraries, even if they depend on tz.h. And they don't introduce any other dependencies.

Looking at this outside of CMake, all it takes to use any of these extras is to simply include it. No source file needs to be compiled. No extra utilities need to be downloaded (not even the IANA tz database).

@mellery451
Copy link
Contributor

sounds good - I'll see if I can formulate a patch that creates the "extras" target for cmake users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants