Skip to content

build: add windows build and release#158

Merged
obnoxxx merged 2 commits intocheckmake:mainfrom
flejz:main
Oct 13, 2025
Merged

build: add windows build and release#158
obnoxxx merged 2 commits intocheckmake:mainfrom
flejz:main

Conversation

@flejz
Copy link
Copy Markdown
Contributor

@flejz flejz commented Oct 6, 2025 •
edited
Loading

Checklist

Not all of these might apply to your change but the more you are able to check
the easier it will be to get your contribution merged.

  • CI passes
  • Description of proposed change
  • Documentation (README, docs/, man pages) is updated

Purpose:
Compile for windows so there is official support.
Release tested and working for targets:

GOOS=windows
GOARCH=amd64
GOARCH=arm64

https://github.com/flejz/checkmake/actions/runs/18276474308/job/52029646621

package cloud build is failing but it is out of the scope of this PR

@flejz flejz marked this pull request as ready for review October 6, 2025 09:46
obnoxxx
obnoxxx requested changes Oct 6, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution, @flejz !

In general, it sounds good to add support for building on Windows.

Since I don't have a Windows machine to build on, it would be nice if you could add a ci test to build and test on Windows (To verify your changes and protect from regressions). Currently, the CI only tests on Linux.

Additionally, you seem to have done a 0.2.3release in your own fork and added a release notes file to this PR. I would request you to hold off such change until we do an official release here. And please remove the release file from this PR and remove mention of release from the commit message.

I'll need to review the changes to packages.yml thoroughly and will get back.

@flejz
Copy link
Copy Markdown
Contributor Author

flejz commented Oct 7, 2025

The "release" I made was only really to test the pipeline. I will remove it from my fork given the intention of keeping it in its official repo.

Will do the changes requested asap.

@flejz
Copy link
Copy Markdown
Contributor Author

flejz commented Oct 11, 2025

@obnoxxx now app is building and testing also o Windows with minimal changes in the CI pipeline.

https://github.com/flejz/checkmake/actions/runs/18430886793/job/52518037763

Waiting feedback.

flejz
flejz commented Oct 11, 2025
View reviewed changes
Comment thread Makefile
flejz
flejz commented Oct 11, 2025
View reviewed changes
Comment thread .github/workflows/ci.yml
obnoxxx
obnoxxx requested changes Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

Thank you for the update @flejz !

This looks a lot better now.

But apart from the request for the Makefile change inline below I have a few general requests:

  • please use git rebase origin/main instead of git merge.
  • would you please sign off your commits by adding a Signed-off-by line (E. g. using git commit -s?
  • Please squash your two commits so that there is no separate commit for the removal of the release notes added in the first commit.

Comment thread Makefile Outdated
obnoxxx
obnoxxx requested changes Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

one more request.

Comment thread .github/workflows/ci.yml
@flejz
Copy link
Copy Markdown
Contributor Author

flejz commented Oct 13, 2025

Will apply the changes asap then let you know.

@flejz flejz requested a review from obnoxxx October 13, 2025 16:55
obnoxxx
obnoxxx approved these changes Oct 13, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@obnoxxx obnoxxx left a comment

Choose a reason for hiding this comment

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

Thank you for the update, @flejz !

This looks good now! 🎉
Thank you for your contribution!

@obnoxxx obnoxxx merged commit ba0a410 into checkmake:main Oct 13, 2025
3 checks passed
@obnoxxx obnoxxx mentioned this pull request Oct 14, 2025
Open
@flejz
Copy link
Copy Markdown
Contributor Author

flejz commented Oct 21, 2025 •
edited
Loading

@obnoxxx when can we expect a new release that includes all latest changes + official Windows support?

@flejz flejz mentioned this pull request Jan 17, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@obnoxxx obnoxxx obnoxxx approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@flejz @obnoxxx