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

chore: Merge with upstream #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

JFreegman
Copy link
Member

@JFreegman JFreegman commented Nov 16, 2024

Upstream is currently staying up-to-date with Zig master instead of a major release. This PR will fix the build for Zig master (2024-11-14), but I think we should stop updating the API until the next major release, and then stick with that.


This change is Reviewable

@JFreegman JFreegman added the chore Updating grunt tasks etc; no production code change label Nov 16, 2024
@freylax
Copy link
Collaborator

freylax commented Nov 17, 2024

Dear JFreegman,
I did some improvements for the c-toxcore-build-with-zig and c-toxcore-zig repos:

  • using a specific Zig Version for CI (0.14.0 , 2024.10.0-mach)
  • local node test works now on my box and CI too.
    I would like that thees changes go in your PR.

@@ -1,15 +1,17 @@
.{
.name = "zig-toxcore-c",
.name = "c-toxcore-zig",
Copy link
Member

Choose a reason for hiding this comment

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

Why the rename?

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, the original repository name at nodecum is c-toxcore-zig...
Should I rename the original one? This could mess things up or?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean how would github behave if I rename nodecum/c-toxcore-zig which is the origin of the fork of TokTok/zig-toxcore-c.

@pull-request-attention pull-request-attention bot assigned JFreegman and iphydf and unassigned freylax Nov 17, 2024
@JFreegman
Copy link
Member Author

Dear JFreegman, I did some improvements for the c-toxcore-build-with-zig and c-toxcore-zig repos:

* using a specific Zig Version for CI (0.14.0 , 2024.10.0-mach)

* local node test works now on my box and CI too.
  I would like that thees changes go in your PR.

Just out of curiosity, why don't the bindings simply link libtoxcore as a system lib?

@freylax
Copy link
Collaborator

freylax commented Nov 19, 2024

Linking to libtoxcore as a system lib would be possible for sure. However then you would have to install (and maybe build) it yourself. Maybe we could have both options, to use a installed system libtoxcore or a static zig build libtoxcore with a tight coupling to the used revision and building of the lib.
I realized the last option because it gets you a smooth zig build experience, but I see that the other approach is feasible as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Updating grunt tasks etc; no production code change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants