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

Time to retire USE_DATE_TIME_PRE_1_33_FACET_IO #61

Closed
jeking3 opened this issue Dec 27, 2017 · 11 comments
Closed

Time to retire USE_DATE_TIME_PRE_1_33_FACET_IO #61

jeking3 opened this issue Dec 27, 2017 · 11 comments
Assignees

Comments

@jeking3
Copy link
Contributor

jeking3 commented Dec 27, 2017

1.33.0 came out a long time ago. Time to retire the pre-1.33.0 code paths.

@eldiener
Copy link
Contributor

I think you should query the developers mailing list regarding this potential removal.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 17, 2018

There was no feedback indicating this should not be done.

@eldiener
Copy link
Contributor

I will remove it and update the 'develop' branch after I do.

@eldiener
Copy link
Contributor

It looks to me that all sorts of tests are only being run when USE_DATE_TIME_PRE_1_33_FACET_IO is being defined. These includes tests for:

  • date_time_wide_streaming
  • date_time_pre_133_facet
  • date_time_serialization

I do not see the same tests being run without USE_DATE_TIME_PRE_1_33_FACET_IO being defined. I feel that I am missing something in the bjam file for the tests but I can not see what it is.

This suggests to me that we can not remove the code which contains USE_DATE_TIME_PRE_1_33_FACET_IO.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 18, 2018

Hmm, interesting. Well I suggest we leave this here for now and revisit it once the trac backlog is drained a little more, or someone can pick it up.

It would seem that date_time_pre_133_facet would be unnecessary entirely based on the name alone.. not sure about the others.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 21, 2018

It's possible that serialization integration only happens pre-1.33 because of the streaming and traits introduced in 1.33... While enabling UBSAN builds I found I had to disable the serialization tests which only run in pre-1.33 compatibility mode. Might be more reason to remove that legacy code...

@eldiener
Copy link
Contributor

If serialization needs the pre-1.33 code I do not see how we can remove that code without saying that serialization does not work for iostreams.

@eldiener
Copy link
Contributor

Sorry I meant to say:

If serialization needs the pre-1.33 code I do not see how we can remove that code without saying that serialization does not work for date_time.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 22, 2018

True, will come back to it eventually and see.

@JeffGarland
Copy link
Collaborator

So I'm planning to remove support for this in boost 1.73 -- the time has seriously come. I've made an initial commit that removes the macros the force this mode for long dropped compilers. I posted a warning on both the mailing lists: crickets.

https://lists.boost.org/boost-users/2020/03/90303.php
https://lists.boost.org/Archives/boost/2020/03/248334.php

I have a 'dt_nolib' branch which I'm going to put onto develop shortly -- removing the library build. That changes the testing quite dramatically and as part of that I've dropped all the pre-1.33 tests. The code is not removed, yet -- but that will be soon.

@JeffGarland
Copy link
Collaborator

This has been merged to master for release in 1.73

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

No branches or pull requests

3 participants