Skip to content

Conversation

@Oliver-Quail
Copy link
Collaborator

@Oliver-Quail Oliver-Quail commented Jul 28, 2025

Description

  • Bug fix for CPP support with new file structure
  • Updates to install steps in README.md

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation (update or new)

How Has This Been Tested?

This has been tested once completed. Then with a recloning and install with both producing identical correct results. Other languages have tested with changes producing know issues.
Please note when testing you will need to run the following command in addition to the other setup commands

git submodule update --init 

Testing Checklist

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link

@sam-stajnko sam-stajnko left a comment

Choose a reason for hiding this comment

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

Hi @Oliver-Quail ,
I have looked at the changes you have made in this PR and I can't see anything that stands out as an issue. I can confirm that the following changes have been made and all changes are performing their intended purpose:

  • The documentation in the README has been updated since the user no longer needs to change directory into SplashkitOnline/Browser_IDE/.
  • All code referencing the 'external' folder has been modified since the contents of this folder have been moved.
  • Both the Python and JS setup scripts no longer download the files SplashKitBackendWASMCPP.js and SplashKitBackendWASMCPP.worker.js. I assume this is because they are now included locally in the runtimes/cxx/bin directory?

I have tested the above changes by building the Splashkit Online project on my own machine and making sure the CPP functionality performs its intended purpose, without crashing or bugs.

I can say with confidence that no build or runtime errors were encountered during the time testing. For this reason, I have no problem approving this PR and moving it to the second round of peer review. Keep up the good work Oliver.

Regards,
Sam Stajnko.

@Oliver-Quail
Copy link
Collaborator Author

Hi @Oliver-Quail , I have looked at the changes you have made in this PR and I can't see anything that stands out as an issue. I can confirm that the following changes have been made and all changes are performing their intended purpose:

* The documentation in the README has been updated since the user no longer needs to change directory into SplashkitOnline/Browser_IDE/.

* All code referencing the 'external' folder has been modified since the contents of this folder have been moved.

* Both the Python and JS setup scripts no longer download the files SplashKitBackendWASMCPP.js and SplashKitBackendWASMCPP.worker.js. I assume this is because they are now included locally in the runtimes/cxx/bin directory?

I have tested the above changes by building the Splashkit Online project on my own machine and making sure the CPP functionality performs its intended purpose, without crashing or bugs.

I can say with confidence that no build or runtime errors were encountered during the time testing. For this reason, I have no problem approving this PR and moving it to the second round of peer review. Keep up the good work Oliver.

Regards, Sam Stajnko.

Hi Sam,
With regards to your third point, that is correct, they have been added to the main repo now

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.

3 participants