-
Notifications
You must be signed in to change notification settings - Fork 52
build: add windows build and release #158
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
Conversation
obnoxxx
left a comment
There was a problem hiding this 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.
|
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. |
|
@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. |
obnoxxx
left a comment
There was a problem hiding this 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/maininstead ofgit merge. - would you please sign off your commits by adding a
Signed-off-byline (E. g. usinggit 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.
obnoxxx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more request.
|
Will apply the changes asap then let you know. |
Signed-off-by: flejz <flejz@protonmail.com>
Signed-off-by: flejz <flejz@protonmail.com>
obnoxxx
left a comment
There was a problem hiding this 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 when can we expect a new release that includes all latest changes + official Windows support? |
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.
Purpose:
Compile for windows so there is official support.
Release tested and working for targets:
https://github.com/flejz/checkmake/actions/runs/18276474308/job/52029646621