Skip to content

Conversation

@faze-geek
Copy link
Contributor

@faze-geek faze-geek commented Dec 18, 2024

Description

Continuation of #360

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Requires documentation updates

Checklist

  • I have read the contribution guide recently

@codecov
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.84%. Comparing base (30aa130) to head (4322fb6).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #398   +/-   ##
=======================================
  Coverage   70.84%   70.84%           
=======================================
  Files           9        9           
  Lines        3529     3529           
=======================================
  Hits         2500     2500           
  Misses       1029     1029           

@anutosh491
Copy link
Collaborator

anutosh491 commented Dec 18, 2024

there's some uncertainty here, do we need to have use_cling=off everywhere ?

As in if we say repl is on by default, I am guessing that also implies cling is off by default ? (Is this what we were trying to achieve @vgvassilev ?)

@faze-geek
Copy link
Contributor Author

Yes that was mentioned in this comment as well, we can go ahead and remove all DUSE_CLING=off instances.

@vgvassilev
Copy link
Contributor

there's some uncertainty here, do we need to have use_cling=off everywhere ?

As in if we say repl is on by default, I am guessing that also implies cling is off by default ? (Is this what we were trying to achieve @vgvassilev ?)

Yes

@vgvassilev
Copy link
Contributor

Yes that was mentioned in this comment as well, we can go ahead and remove all DUSE_CLING=off instances.

Yes. We should do that.

@vgvassilev vgvassilev requested a review from mcbarton December 18, 2024 19:04
@mcbarton
Copy link
Collaborator

@faze-geek let me know when you've removed all references to use_cling=off and i'll review.

@faze-geek
Copy link
Contributor Author

@mcbarton You can have a look right now.

Copy link
Collaborator

@anutosh491 anutosh491 left a comment

Choose a reason for hiding this comment

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

I think this looks fine to me !

@mcbarton
Copy link
Collaborator

@faze-geek You need to make changes in the recently introduced emscripten build instructions file.

@faze-geek
Copy link
Contributor Author

@faze-geek You need to make changes in the recently introduced emscripten build instructions file.

Will make those changes quick. Thanks !

@mcbarton mcbarton merged commit cb640db into compiler-research:main Dec 19, 2024
46 checks passed
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.

4 participants