Skip to content
This repository has been archived by the owner. It is now read-only.

#1150 - Add js test sample #1386

Closed
wants to merge 1 commit into from
Closed

#1150 - Add js test sample #1386

wants to merge 1 commit into from

Conversation

@Patouche
Copy link

@Patouche Patouche commented Jun 15, 2019

Context

Provide test for samples js "hello-js" (see #1150 for further details)

Made during https://hack-commit-pu.sh/ event.

Thanks a lot to @aalmiray for his participation.

Copy link
Member

@eskatos eskatos left a comment

Thank you for the PR @Patouche.
It passed the Travis checks on Java 11. The Java 8 setup is currently broken, please ignore it.

I left a few comments.
Could you also please add a test for the sample in HelloJsSampleTest? I think running the test task would be enough.

Before we can accept your contribution, you need to sign-off your commits, see the failed DCO check on Github https://github.com/gradle/kotlin-dsl/pull/1386/checks?check_run_id=149137955 for how to proceed.

Loading

}
}

node {
Copy link
Member

@eskatos eskatos Jun 17, 2019

Choose a reason for hiding this comment

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

iirc, this is the node project extension, not a task. I would move its configuration out of the tasks {} block to make this clear. Or, if setting the node version isn't required for this sample it could simply be removed, your call.

Loading

version = "10.16.0"
}

karma {
Copy link
Member

@eskatos eskatos Jun 17, 2019

Choose a reason for hiding this comment

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

this is also a project extension, please move its configuration out of the tasks {} block

Loading

outputs.dir(outputDir)
doLast {
val fromJars = configurations.testCompileClasspath.get()
.filter { it.name.matches(Regex(".*\\.jar")) }
Copy link
Member

@eskatos eskatos Jun 17, 2019

Choose a reason for hiding this comment

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

Suggested change
.filter { it.name.matches(Regex(".*\\.jar")) }
.filter { it.extension == "jar") }

Loading

Signed-off-by: Patrick Allain <allain.pat@gmail.com>

# Please enter the commit message for your changes. Lines starting
# with '#' will be kept; you may remove them yourself if you want to.
# An empty message aborts the commit.
#
# Date:      Sat Jun 15 17:37:07 2019 +0200
#
# On branch master
# Your branch is up to date with 'origin/master'.
#
# Changes to be committed:
#	modified:   samples/hello-js/build.gradle.kts
#	new file:   samples/hello-js/src/main/kotlin/samples/Greeter.kt
#	new file:   samples/hello-js/src/test/kotlin/samples/GreeterTest.kt
#
@eskatos
Copy link
Member

@eskatos eskatos commented Nov 6, 2019

The hello-js sample uses the outdated kotlin-js plugin that is superseded by Kotlin MPP Javascript support.
Closing as dropped.
Thanks for the effort @Patouche, sorry this cannot be merged.

Loading

@eskatos eskatos closed this Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants