Skip to content

fix: CMake package and pkgconfig rework#916

Merged
henryiii merged 26 commits into
CLIUtils:mainfrom
phlptp:pkg_tests
Sep 15, 2023
Merged

fix: CMake package and pkgconfig rework#916
henryiii merged 26 commits into
CLIUtils:mainfrom
phlptp:pkg_tests

Conversation

@phlptp
Copy link
Copy Markdown
Collaborator

@phlptp phlptp commented Aug 15, 2023

add tests that exercise the CMake module and pkg-config files

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 15, 2023 •
edited
Loading

Codecov Report

Patch has no changes to coverable lines.

📢 Thoughts on this report? Let us know!.

@phlptp
Copy link
Copy Markdown
Collaborator Author

phlptp commented Aug 18, 2023

FIxes #911

phlptp
phlptp commented Aug 18, 2023
View reviewed changes
Comment thread .github/workflows/tests.yml
cmake -S . -B build \
-DCMAKE_CXX_STANDARD=${{matrix.std}} \
-DCLI11_SINGLE_FILE_TESTS=OFF \
-DCLI11_EXAMPLES=OFF \
Copy link
Copy Markdown
Collaborator Author

@phlptp phlptp Aug 18, 2023

Choose a reason for hiding this comment

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

I think this change is what caused the coverage change, but I don't think the examples should be built or tested during the single file tests, not entirely sure what is going on.

Copy link
Copy Markdown
Collaborator

@henryiii henryiii Sep 15, 2023

Choose a reason for hiding this comment

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

Coverage upload is finicky, it's possible it just had too many jobs not able to report. Going to try a rebuild to see. Otherwise, not too worked about it.

Copy link
Copy Markdown
Collaborator Author

@phlptp phlptp Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I should take another crack at getting back to 100% sometime soon. Probably not in this PR though

Copy link
Copy Markdown
Collaborator

@henryiii henryiii Sep 15, 2023

Choose a reason for hiding this comment

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

The failing lines are

https://github.com/phlptp/CLI11/blob/1cb75471863bb364038de3a5ef8b596694945990/include/CLI/impl/App_inl.hpp#L608-L609

https://github.com/phlptp/CLI11/blob/1cb75471863bb364038de3a5ef8b596694945990/include/CLI/impl/Config_inl.hpp#L250-L251

https://github.com/phlptp/CLI11/blob/1cb75471863bb364038de3a5ef8b596694945990/include/CLI/impl/Config_inl.hpp#L342-L343

I think they were uncovered before, but for some reason they are now counting as a slightly higher percentage? Coverage is weird unless it's 100%. :/

Not worried about it for this PR.

@phlptp phlptp requested a review from henryiii August 18, 2023 03:08
@henryiii henryiii closed this Sep 15, 2023
@henryiii henryiii reopened this Sep 15, 2023
@henryiii henryiii changed the title add tests of the CMake package and pkgconfig fix: CMake package and pkgconfig rework Sep 15, 2023
@henryiii henryiii merged commit c071cb6 into CLIUtils:main Sep 15, 2023
@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 Sep 15, 2023
@Porkepix Porkepix mentioned this pull request Feb 5, 2024
Merged
@j-rivero j-rivero mentioned this pull request Feb 26, 2024
Merged
phlptp pushed a commit that referenced this pull request Mar 11, 2024
Restore non-arch dependant path for the pkgconfig file (#1012)
852fc94 
Originally setup at #635 but
reverted in #916 without any
specific reason that I can find.
@phlptp phlptp deleted the pkg_tests branch October 20, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henryiii henryiii Awaiting requested review from henryiii

Assignees

No one assigned

Labels

needs changelog Hasn't been added to the changelog yet needs README Needs to be mentioned in the README

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phlptp @henryiii