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) move moonscript into separate C file #1670

Merged
merged 2 commits into from
Nov 5, 2021

Conversation

joshgoebel
Copy link
Collaborator

@joshgoebel joshgoebel commented Nov 5, 2021

Feels like every language should have their own C file in api... so I wonder if this should be broken out?

@joshgoebel
Copy link
Collaborator Author

Would this be good? If so I can do the same with Fennell...

@joshgoebel joshgoebel changed the title (chore) more moonscript into separate C file (chore) move moonscript into separate C file Nov 5, 2021
@@ -1447,7 +1447,7 @@ static void squirrel_open_builtins(HSQUIRRELVM vm)
sq_poptop(vm);
}

static void initAPI(tic_core* core)
static void initLuaAPI(tic_core* core)
Copy link
Owner

Choose a reason for hiding this comment

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

It looks like the function was accidentally renamed here, there should be no mention of Lua in the Squirrel module.

src/api/wren.c Outdated
@@ -1395,7 +1395,7 @@ static WrenForeignMethodFn bindForeignMethod(
return foreignTicMethods(fullName);
}

static void initAPI(tic_core* core)
static void initLuaAPI(tic_core* core)
Copy link
Owner

Choose a reason for hiding this comment

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

Same here, please fix if not difficult.
Thank you.

@joshgoebel
Copy link
Collaborator Author

Yeah, no problem. I'll fix this up and make sure the tests are green. Just didn't want to throw more work at this if you aren't ok with the change in general.

Copy link
Owner

@nesbox nesbox 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

@nesbox nesbox merged commit 666705d into nesbox:master Nov 5, 2021
nesbox added a commit that referenced this pull request Nov 5, 2021
nesbox added a commit that referenced this pull request Nov 5, 2021
@joshgoebel
Copy link
Collaborator Author

Yeah, not sure this was 100% ready yet. :) You got to it too fast.

@nesbox
Copy link
Owner

nesbox commented Nov 5, 2021

ok, waiting... :)

@joshgoebel
Copy link
Collaborator Author

But you already merged it... did you get it working or should I just open a new PR? :-)

@nesbox
Copy link
Owner

nesbox commented Nov 5, 2021

Honestly, I don't know what to do in this case :)

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.

2 participants