Skip to content

Conversation

@dsanders11
Copy link
Member

@dsanders11 dsanders11 commented Nov 8, 2023

  • The original gyp build in this repo has broken - since gyp is more or less unmaintained these days, switch to CMake
    • I tried to replicate the gyp config in CMake
  • Move from AppVeyor to GitHub Actions - it will be simpler to maintain going forward
  • Refactor how the version is determined for printing help - rather than hardcoding it in version.h
    • By fetching it from the product version, we can dogfood and stamp the version into the exe after building
  • Use semantic-release to create GitHub releases
    • Paired with the refactor, we can stamp the correct version into the exe at build time with no manual work
  • Drops support for Windows XP
    • Visual Studio 2017 deprecated support and CI is on Visual Studio 2022, so it's not feasible to continue supporting it

@dsanders11 dsanders11 force-pushed the ci-github-actions branch 13 times, most recently from 6cfdefd to 80c33de Compare November 9, 2023 08:54
@dsanders11 dsanders11 changed the title ci: use GitHub Actions refactor!: build with CMake and release with semantic-release Nov 9, 2023
@dsanders11 dsanders11 force-pushed the ci-github-actions branch 6 times, most recently from 53a07d8 to ced98af Compare November 9, 2023 22:18
@dsanders11 dsanders11 changed the title refactor!: build with CMake and release with semantic-release feat!: build with CMake and release with semantic-release Nov 9, 2023
BREAKING CHANGE: Drops Windows XP support
Copy link
Contributor

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

You can store the data in a std::vector to avoid manual new/delete.

@dsanders11 dsanders11 requested a review from zcbenz November 10, 2023 00:43
Copy link
Contributor

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

Code looks good to me.

@dsanders11 dsanders11 marked this pull request as ready for review November 10, 2023 01:12
@dsanders11 dsanders11 requested a review from a team as a code owner November 10, 2023 01:12
Copy link
Member

@VerteDinde VerteDinde left a comment

Choose a reason for hiding this comment

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

LGTM - we should disable AppVeyor for this repo and kill that check

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.

4 participants