feat: Consider emscripten a UNIX platform#4168
Conversation
github-actions
Bot
added
the
release::enhancements
jjerphan
force-pushed
the
feat/consider-emscripten-targets-to-be-unix-platform
branch
from
e7f53e6 to
59e0314
Compare
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan
force-pushed
the
feat/consider-emscripten-targets-to-be-unix-platform
branch
from
59e0314 to
6d97a01
Compare
| return std::move(override_version).value(); | ||
| } | ||
| // 4.x is assumed to be the latest version used by emscripten-forge. | ||
| return { "4.x" }; |
There was a problem hiding this comment.
Could this be the same as the emscripten-abi version from the solve result? Otherwise not sure it makes sense here.
There was a problem hiding this comment.
Yes, as discussed privately, this version is just placed but not actually used (AFAIK).
I'll add a comment to explain and motivates those changes in details.
There was a problem hiding this comment.
If the version is not used, it should be set to something that cannot be considered as a correct version. The comment explaining this is part of the caller instead of this function (which makes sense since it's part of a more global explanation), so maybe it would be better to pass the default version as an argument of this function?
There was a problem hiding this comment.
As discussed privately, we do not need __emscripten, so let's keep something simple.
See fce4662.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4168 +/- ##
=======================================
Coverage 52.09% 52.10%
=======================================
Files 239 239
Lines 28715 28717 +2
Branches 3015 3016 +1
=======================================
+ Hits 14960 14962 +2
Misses 13752 13752
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan
changed the title
emscripten-* targets to be UNIX platformemscripten a UNIX platform
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
jjerphan
deleted the
feat/consider-emscripten-targets-to-be-unix-platform
branch
3 participants
Description
Type of Change
Checklist
pre-commit run --alllocally in the source folder and confirmed that there are no linter errors.