Skip to content

Conversation

@AnticliMaxtic
Copy link
Contributor

rename enum class parameter WINDOWS in detail::Classifier to WINDOWS_STYLE to remove conflicts with cmake default definitions when using Visual Studio generator (/D_WINDOWS).

This is causing build conflicts for me in projects I include this library.

@AnticliMaxtic
Copy link
Contributor Author

@henryiii Any idea when this might get merged into master?

@henryiii
Copy link
Collaborator

henryiii commented Apr 2, 2021

Hopefully quite soon. I have been out for a while (have a one-month old daughter), and the CI needs some work before I can finish up and get 2.0 out. I should start having time fairly soon, catching up on a lot of things now.

@henryiii henryiii closed this Apr 2, 2021
@henryiii henryiii reopened this Apr 2, 2021
@AnticliMaxtic
Copy link
Contributor Author

cool, just checking in on it. Congratulations! I know how much work that can be

@henryiii
Copy link
Collaborator

henryiii commented Apr 2, 2021

CI's still a little broken, but not this broken:

/__w/1/s/tests/AppTest.cpp:1969:89: error: 'WINDOWS' is not a member of 'CLI::detail::Classifier'
     EXPECT_THROW(CLI::detail::AppFriend::parse_arg(&app, args, CLI::detail::Classifier::WINDOWS), CLI::HorribleError);

Looks like there still are WINDOWS usages in the tests. git grep "\bWINDOWS\b" should help find them all. Think you can fix this? Will probably take longer for me to get around to it.

@codecov
Copy link

codecov bot commented Apr 3, 2021

Codecov Report

Merging #563 (ea91c7f) into master (34c4310) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ea91c7f differs from pull request most recent head 214ea20. Consider uploading reports for the commit 214ea20 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master      #563   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines         3780      3780           
=========================================
  Hits          3780      3780           
Impacted Files Coverage Δ
include/CLI/App.hpp 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa423c4...214ea20. Read the comment docs.

…WINDOWS_STYLE' to remove conflicts with cmake default definitions
@henryiii henryiii force-pushed the AnticliMaxtic/correct-windows-default-clash branch from ea91c7f to 214ea20 Compare April 4, 2021 04:38
@henryiii henryiii merged commit 3eb5e1e into CLIUtils:master Apr 4, 2021
@github-actions github-actions bot added needs changelog Hasn't been added to the changelog yet needs README Needs to be mentioned in the README labels Apr 4, 2021
@AnticliMaxtic AnticliMaxtic deleted the AnticliMaxtic/correct-windows-default-clash branch April 4, 2021 13:28
@henryiii henryiii removed needs README Needs to be mentioned in the README needs changelog Hasn't been added to the changelog yet labels Apr 8, 2021
@henryiii henryiii added this to the v2.0 milestone Jun 24, 2021
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.

2 participants