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

js: write PerCharacterEscaper.js #9

Merged
merged 3 commits into from
Sep 9, 2023
Merged

js: write PerCharacterEscaper.js #9

merged 3 commits into from
Sep 9, 2023

Conversation

jknack
Copy link
Collaborator

@jknack jknack commented Aug 23, 2023

  • There is a lot more code that can be reused
  • Key method points are: codePointAt, codePoints, charCount
  • Seems jsTests are not running from gradle, but I couldn't figure it out how to do it. Tests run from IDEA.

Let me know if you want to go harder with code reuse in this class and just makes platform dependent the 3/4 functions I mentioned.

Finally, let me know if I screw something.. first time with kmm JS

- There is a lot more code that can be reused
- Key method points are: `codePointAt`, `codePoints`, `charCount`
- Seems jsTests are not running from gradle, but I couldn't figure it out how to do it

Let me know if you want to go `harder` with code reuse in this class and just makes platform dependent the 3/4 functions I mentioned.
@nedtwigg
Copy link
Member

Excellent!

jsTests are not running

./gradlew :selfie-lib:tasks --all
...
Verification tasks
------------------
allTests - Runs the tests for all targets and create aggregated report
jsTest - Run JS tests for all platforms
jvmTest - Runs the tests of the test test run.

KMM doesn't follow the normal Gradle convention, it actually doesn't have a test task out of the box at all, but I added it for convenience:

// the normal `test` task doesn't work for multiplatform projects
tasks.create('test') {
dependsOn('jvmTest')
}

harder with code reuse

I would start with test reuse: delete the tests in jsTest and jvmTest, and put the test into commonTest.

In general, I would like to go as hard with the common directory as we can. Much later on I'd like to add native support too (KMM is pretty immature there right now, but it's getting better), and the more common code there is, the easier that will be.

@jknack
Copy link
Collaborator Author

jknack commented Aug 23, 2023

I would start with test reuse: delete the tests in jsTest and jvmTest, and put the test into commonTest.

will that run all the targets (jvm/js) from it?

@nedtwigg
Copy link
Member

Correct.

@nedtwigg
Copy link
Member

If you break the ArrayMapTest on purpose, you'll see that jsTest and jvmTest are both broken, because ArrayMapTest lives in common.

@jknack
Copy link
Collaborator Author

jknack commented Aug 23, 2023

moved test to common

@nedtwigg
Copy link
Member

nedtwigg commented Sep 9, 2023

This works great, we can revisit it in the future if we want to, but we might not need to.

@nedtwigg nedtwigg merged commit 61fba2b into main Sep 9, 2023
7 checks passed
@nedtwigg nedtwigg deleted the percharesscaper.js branch September 9, 2023 17:28
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.

2 participants