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

feat!: build with CMake and release with semantic-release #118

Merged
merged 6 commits into from
Nov 15, 2023

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.

src/main.cc Outdated Show resolved Hide resolved
src/main.cc Outdated Show resolved Hide resolved
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.

3 participants