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

Improvement for JS ecosystem #2270

Merged
merged 7 commits into from Apr 8, 2024
Merged

Improvement for JS ecosystem #2270

merged 7 commits into from Apr 8, 2024

Conversation

gausie
Copy link
Contributor

@gausie gausie commented Apr 8, 2024

Currently kolmafia can't be imported outside of a KoLmafia environment because ESM knows that the named exports aren't actually there. I would like to do this so I can share code between the web and a mafia script, knowing that I will not actually execute mafia-specific code. This fixes that

@gausie gausie requested a review from a team as a code owner April 8, 2024 11:49
Copy link

codecov bot commented Apr 8, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 37.70%. Comparing base (81a0fd4) to head (4cbd87a).
Report is 2 commits behind head on main.

❗ Current head 4cbd87a differs from pull request most recent head 8cdb247. Consider uploading reports for the commit 8cdb247 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2270      +/-   ##
============================================
+ Coverage     37.63%   37.70%   +0.06%     
- Complexity    19938    19970      +32     
============================================
  Files          1110     1111       +1     
  Lines        169787   169839      +52     
  Branches      35866    35857       -9     
============================================
+ Hits          63905    64043     +138     
+ Misses        95752    95664      -88     
- Partials      10130    10132       +2     
Files Coverage Δ
...rceforge/kolmafia/textui/TypescriptDefinition.java 90.45% <60.00%> (-2.29%) ⬇️

... and 12 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f1ac30...8cdb247. Read the comment docs.

jaadams5
jaadams5 previously approved these changes Apr 8, 2024
Copy link
Contributor

@jaadams5 jaadams5 left a comment

Choose a reason for hiding this comment

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

Not that I totally understand what is happening but I trust that it works and doesn't break anything "normal" KolMafia users use.

@gausie
Copy link
Contributor Author

gausie commented Apr 8, 2024

Without usage statistics I would imagine most KoLmafia users execute JS scripts at least once day, which this could affect if it were wrong.

@gausie gausie merged commit a60482e into main Apr 8, 2024
6 checks passed
@gausie gausie deleted the js-ecosystem branch April 8, 2024 16:26
@gausie
Copy link
Contributor Author

gausie commented Apr 8, 2024

Small mistake in the github action, I will push to main to fix.

@Veracity0
Copy link
Contributor

Without usage statistics I would imagine most KoLmafia users execute JS scripts at least once day, which this could affect if it were wrong.

I would be astonished if this were true.

Perhaps you mean "most KoLmafia users who are on Discord" - which I am sure is a minority of all KoLmafia users.

@gausie
Copy link
Contributor Author

gausie commented Apr 9, 2024 via email

@midgleyc
Copy link
Member

midgleyc commented Apr 9, 2024

Without usage statistics I expect most KoLmafia users don't execute any scripts at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants