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

Changes for Embarcadero C++ clang-based compilers #266

Closed
wants to merge 7 commits into from
Closed

Changes for Embarcadero C++ clang-based compilers #266

wants to merge 7 commits into from

Conversation

eldiener
Copy link
Contributor

No description provided.

@raffienficiaud
Copy link
Member

Please do not get angry at the Travis/Appveyor/Codecov on the ML, nothing works right now and I have my own CI.

@eldiener
Copy link
Contributor Author

I am sorry if i gave the impression that I am angry in any way, whether that involves code coverage or any other issue. There is no rush on seeing if this PR is valid since, as I say in the title, the change is targeted for Boost 1.74 and that is a long way away. I still have more work to do to try to get the Embarcadero C++ clang-based compilers to work properly with Boost.Test but this PR is at least a good first start, but there is no rush to get this merged right now.

… become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.
@raffienficiaud
Copy link
Member

Hi,

Sorry for the late answer:

I am sorry if i gave the impression that I am angry in any way

No worries on my side, I just wanted to point out that some of those CI thinggies are not working as expected. But all is fine on my personal CI, so the PR seems good to go.

I would just like to squash some commits, aggregate some macros and cleanup execution_monitor.ipp. Should be quite fast.

@raffienficiaud raffienficiaud changed the title Changes for Embarcadero C++ clang-based compilers, targeting Boost 1.74 Changes for Embarcadero C++ clang-based compilers May 9, 2020
@raffienficiaud raffienficiaud self-assigned this May 9, 2020
@raffienficiaud raffienficiaud added this to the 1.74 milestone May 9, 2020
@raffienficiaud
Copy link
Member

raffienficiaud commented May 9, 2020

Branch topic/PR-266-Embarcadero-C++-clang-based-compilers is ready to get merged to next. I made a few (minor/cosmetic) edits, it would be good that you have a look as I have no way to check the compiler.

@eldiener
Copy link
Contributor Author

eldiener commented May 9, 2020

Looks fine. I am not sure what adding the 1.74 label or adding to the 1.74 milestone actually does, but as far as changing the title of the PR I am good with that. I am 100% sure that my PR changes will not affect any other compiler than the Embarcadero C++ clang-based compilers, and I still have work to do to test those compilers with this library, but the PR is a good start.

@raffienficiaud
Copy link
Member

Those labels are for my personal (lack of) organization. Thanks for the quick feedback.

@raffienficiaud raffienficiaud added next In "next" branch develop and removed next In "next" branch labels May 9, 2020
@raffienficiaud
Copy link
Member

In develop: I suggest that we monitor the test matrix for some time before it lands to master.

@eldiener
Copy link
Contributor Author

eldiener commented May 9, 2020

In develop: I suggest that we monitor the test matrix for some time before it lands to master.

There is no necessity for this to go to 'master' until you are ready. I will check the regression tests on 'develop' to make sure nothing is amiss. You should really try to get your CI tests working properly to give you peace of mind whenever there is a PR against the 'test' library.

@raffienficiaud
Copy link
Member

You should really try to get your CI tests working properly to give you peace of mind whenever there is a PR against the 'test' library.

I have already an excellent coverage on a private CI with ~ 20 machines that covers most of the Appveyor+Travis builds, and which is must faster that those community tools. My CI is green, this is why it has been merged to develop. Despite that, whatever you do, this is still less than some exotic compiler from the test matrix.

@raffienficiaud
Copy link
Member

in master, thanks!

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.

None yet

2 participants