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

Test BN Compatibility tool and github action #569

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

Conversation

CedricGuillemet
Copy link
Contributor

Runs a list of BabylonNative commit against a range of Babylon.js NPM version to list compatible versions.
The list is then uploaded as an artifact.

Copy link
Contributor

@SergioRZMasson SergioRZMasson left a comment

Choose a reason for hiding this comment

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

LGTM

compatibility.push({tag:tag, hash:hash, npms:compatiblePackageVersions});
}

BRNVersions.forEach((versionToTest) =>{
Copy link
Member

Choose a reason for hiding this comment

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

Is it a goal to be able to run this for every version locally? Would it make more sense for the version to be an input to the script, and then use a GitHub actions matrix thing to be able to run them all in parallel across multiple machines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would be faster to run. And it should also use the result from last run to not re-test everything.

@@ -0,0 +1,35 @@
# Testing BabylonNative Protocol
Copy link
Member

Choose a reason for hiding this comment

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

When should we use this new tool? Any time a new BJS version is released? Maybe add some comments about that in this doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should run :

  • once a week to test new bjs package(s)
  • after each BRN NPM package release


function checkoutAndBuildBN(tag, hash, callback) {
console.log("Git clone.");
execute('git clone https://github.com/BabylonJS/BabylonNative.git', './', (error, stdout, stderr) => {
Copy link
Member

Choose a reason for hiding this comment

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

Oy. Can we use Promises and async/await in this code? It's just a node.js script, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's a Node.js script.

@@ -0,0 +1,29 @@
name: Test BabylonNative Protocol compatibility
Copy link
Contributor

Choose a reason for hiding this comment

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

Like we discussed offline, this isn't just testing the protocol version. Rename folder to TestCompatibility, etc.?

@CedricGuillemet CedricGuillemet changed the title test BN protocol tool and github action test BN Compatibility tool and github action Mar 14, 2023
@CedricGuillemet CedricGuillemet changed the title test BN Compatibility tool and github action Test BN Compatibility tool and github action Mar 14, 2023

function checkoutAndBuildBN(tag, hash, callback) {
console.log("Git clone.");
execute('git clone https://github.com/BabylonJS/BabylonNative.git', './', (error, stdout, stderr) => {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use async/await here to make this more readable?

CedricGuillemet and others added 7 commits May 24, 2023 09:30
…ylonJS#580)

* Agnostic toolset for VS project

* platform tool version

* pragma for std

* Update Modules/@babylonjs/react-native-windows/windows/BabylonReactNative/BabylonReactNative.vcxproj

Co-authored-by: Gary Hsu <[email protected]>

* update windows readme

---------

Co-authored-by: Gary Hsu <[email protected]>
* 1.6.1 SLA update

* removed empty space
* Update BN submodule

* BN #883ba13

* a676d6df117cb6cdebc5c347b8aa37f2fed852fb

* cmake + brn up

* cmake update

* removed comment

* comment

* GraphicsDevice

* graphics init

* android rn 0.71

* back to 0.64
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.

6 participants