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

Suggestion for Windows, CI, and CMake #68

Merged
merged 5 commits into from
Dec 1, 2023
Merged

Conversation

luncliff
Copy link
Contributor

@luncliff luncliff commented Dec 1, 2023

Hi @lfreist , appreciate your great work.
I'm reviewing the internals before suggesting this project to https://github.com/microsoft/vcpkg

These are my ideas for future maintenance. Would you consider it?

Changes

GitHub Actions

  • Bump the actions/checkout's version
  • After Git checkout, fetch Git submodules

CMake

Example

Return 0 for int main(...)

WMI(Windows Management Instrumentation)

In main branch, CI fails because SerialNumber is empty.
Added nullptr check in several points.

* install headers
* install library
* ctor: check member variable `locator`, `service`
* WbemService before query
* SerialNumber may empty
@luncliff luncliff changed the title Suggestion for Windows CI Suggestion for Windows, CI, and CMake Dec 1, 2023
@lfreist
Copy link
Owner

lfreist commented Dec 1, 2023

Hi, thank you for your valuable contribution. The changes indeed increase the quality of HWinfo!

What exactly are your suggestion for future maintenance?

@lfreist lfreist self-requested a review December 1, 2023 20:32
@lfreist lfreist merged commit 5cb31db into lfreist:main Dec 1, 2023
4 of 5 checks passed
@luncliff
Copy link
Contributor Author

luncliff commented Dec 2, 2023 via email

@lfreist
Copy link
Owner

lfreist commented Dec 2, 2023

Yes, absolutely consider it. I'd like to have a first release version and this will be published using vcpkg

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