Geequlim / ECMAScript

Javascript binding for godotengine
MIT License
959 stars 84 forks source link

Refactor repo branches to enable good publish process #165

Open nmerget opened 11 months ago

nmerget commented 11 months ago

We should do some changes:

fire commented 11 months ago

Master should use 4.1 now.

fire commented 11 months ago

I am testing builds on 4.1 Godot Engine https://github.com/V-Sekai/godot/tree/vsk-javascript-4.1

fire commented 11 months ago

It fails to build on Linux etc.



modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:30:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = p_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:31:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:41:7: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
                temp.u.ptr = js_function.javascript_object;
                ~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:42:7: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
                temp.tag = JS_TAG_OBJECT;
                ~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:50:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:51:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:58:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:59:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:72:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.tag = JS_TAG_OBJECT;
        ~~~~~~~^~~~
modules/javascript/thirdparty/quickjs/quickjs_callable.cpp:73:9: error: member reference base type 'JSValue' (aka 'unsigned long long') is not a structure or union
        js_func.u.ptr = js_function.javascript_object;
        ~~~~~~~^~
10 errors generated.
scons: *** [modules/javascript/thirdparty/quickjs/quickjs_callable.android.template_release.arm32.o] Error 1
modules/javascript/thirdparty/quickjs/quickjs_debugger.cpp:82:24: error: out-of-line definition of 'connect' does not match any declaration in 'QuickJSDebugger'
Error QuickJSDebugger::connect(JSContext *ctx, const String &address) {
                       ^~~~~~~```
nmerget commented 11 months ago

Okay, I have several questions:

  1. Why did you merge a branch with a failed pipeline into master? The master branch should be protected by status checks which can't be bypassed to make sure that there is at least one valid branch. The PR should be reverted until the 4.1 is fixed.

  2. Before merging 4.1 to master, we should open a new branch 3.4. I don't know if someone is using this version for a "real" project, but we should make sure that we are still aligned with the major versions of godot.

  3. For https://github.com/V-Sekai/godot, we should make sure that we enable other builds for linux, which require some changes, but this should be another issue.

  4. You changed/reverted a lot of the branch inside my fork. It's pretty hard to resolve what's going on now. Maybe we can rollback everything to make sure everything is tested?

fire commented 11 months ago
  1. The failed pipeline isn't enabled until the ci can be merged. so we cannot see the effects until it is merged
  2. I'll do that immediately. ✅
  3. https://github.com/V-Sekai/godot - I had no way of testing this here because of (1), so I did it on the v-sekai orgination branch
  4. I'll do this in about an hour

a. copy the last 3.4 build to the branch 3.4 ✅ b. the last commit should be 1 commit on top of gd4 next c. I'll rollback in one hour to this layout:

  1. previous master -> 3.4
  2. current master -> 4.1 (I will pretend 4.2 doesn't exist) ✅
  3. you can look at the gd4 for the commits. let me know if that is ok
fire commented 11 months ago

To be clear. I'll rollback master to the last 3.4 in an hour.

fire commented 11 months ago

@nmerget Visual Studio compiler diverged from the godot 3 version and the cicd breaks.

nmerget commented 11 months ago

I reverted my initial branch to compare the differences between it and the current master. And I have some questions:

Why did you change the .github folder again? Basically dropped all my changes and keep the copy/paste from godotengine. Was this your purpose? Furthermore, the pipeline fails in master because of the wrong indent in this file.

In addition, I'm confused why you kept quickjs/quickjs_callable.cpp in this directory. It would make sense to me to:

  1. move it to thirdparty/quickjs to have everything in one place
  2. create a new directory bindings to with another directory quickjs so we separate thirdparty and bindings

--> I think the main task is to fix the pipeline(s) again and align all the branches, and we shouldn't copy/paste things from godotengine unless it makes sense

fire commented 11 months ago

@nmerget I have been giving less effort to this project. @Geequlim I would suggest giving commit access to @nmerget as he's been doing a good job untangling this. Too many things going on.

You have my blessing to maintain the project.