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

Build libDifferent CI #917

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Build libDifferent CI #917

merged 1 commit into from
Jan 17, 2025

Conversation

Stefterv
Copy link
Collaborator

@Stefterv Stefterv commented Jan 15, 2025

Closes #913

This GitHub Action will:

@hx2A
Copy link
Collaborator

hx2A commented Jan 16, 2025

I'm not the best person to review github actions, but from what I know about them, this looks good to me.

One more change I'd suggest is that we add the compiled library file to .gitignore to discourage anyone from including them in a commit or future PR. I realize this github action would overwrite library files created by PR submitters and adding it to .gitignore doesn't prevent anyone from overriding the ignore directive. I still think it would be a good idea to include it there though.

@Stefterv
Copy link
Collaborator Author

Yeah I thought about that but then the ci for the pull request previews wouldn't work.

As example, you submitted a change and I would like to test that change, then I would need a build with the new binary in Processing to make sure it works. I think the only way would be to allow the binary to be part of the PR

@hx2A
Copy link
Collaborator

hx2A commented Jan 16, 2025

Yeah I thought about that but then the ci for the pull request previews wouldn't work.

As example, you submitted a change and I would like to test that change, then I would need a build with the new binary in Processing to make sure it works. I think the only way would be to allow the binary to be part of the PR

Oh, I see. You need the compiled binary to be in the PR so the CI testing will work. There's no way for the CI stuff to compile it itself, before running tests?

@Stefterv
Copy link
Collaborator Author

Good you ask, I just realized only macOS can export for macOS so maybe it can just be part of the build chain.

I was under the impression that libDifferent needed to be included on other platforms too

In any case core is compiled and posted on Linux and I'd prefer not to change that.

@hx2A
Copy link
Collaborator

hx2A commented Jan 16, 2025

OK, good we talked this through and understand how everything will work moving forward.

Copy link
Collaborator

@hx2A hx2A left a comment

Choose a reason for hiding this comment

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

Looks good! This will compile the native thinkdifferent library, needed for macOS computers

@Stefterv Stefterv merged commit 55c7736 into main Jan 17, 2025
13 checks passed
@Stefterv Stefterv deleted the different-ci branch January 17, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a CI/CD stage for ThinkDifferent class
2 participants