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

Make classDirectory+dependencyClasspath configurable #73

Merged
merged 1 commit into from Jul 5, 2016

Conversation

magro
Copy link
Contributor

@magro magro commented Jul 5, 2016

As mentioned in #63 (comment) one can hack around the hardcoded config that is used for classDirectory and dependencyClasspath (both are taken from the Compile config).

This change allows the user to set

classDirectory in Jmh := (classDirectory in Test).value
dependencyClasspath in Jmh := (dependencyClasspath in Test).value

so that jmh will use the appropriate values.

Not sure if this change provides enough value to be merged, at least it seems to be cleaner when having benchmark code in the Test config.

As mentioned in [sbt#63](sbt#63 (comment))
one can hack around the hardcoded config that is used for `classDirectory`
and `dependencyClasspath` (both are taken from the `Compile` config).

This change allows the user to set

```
classDirectory in Jmh := (classDirectory in Test).value
dependencyClasspath in Jmh := (dependencyClasspath in Test).value
```

so that jmh will use the appropriate values.
@ktoso
Copy link
Member

ktoso commented Jul 5, 2016

I think that's a great PR, thanks for prepping it :)

@magro
Copy link
Contributor Author

magro commented Jul 5, 2016

Great you like it! In #63 (comment) I actually left out some details regarding module dependencies (mixed "compile->test" + "test->test") that can be solved with this PR as I just realized :-)

@ktoso ktoso merged commit bd15eb8 into sbt:master Jul 5, 2016
@ktoso
Copy link
Member

ktoso commented Jul 5, 2016

Thanks :)

I guess we should document now "if you need test dependencies"... would you be able to make your comment into a snippet in the readme?

@magro
Copy link
Contributor Author

magro commented Jul 5, 2016

To me it still seems to be a hack and would need further adjustments to be
considered a solution, although I'm not sure what this would be in detail
:-) If you think it's ok as it is I can send a PR for the readme.

Konrad Malawski notifications@github.com schrieb am Di., 5. Juli 2016,
18:39:

Thanks :)

I guess we should document now "if you need test dependencies"... would
you be able to make your comment into a snippet in the readme?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#73 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAD7HWAoXbTGGMHJQp0iE6FJGofpBn9_ks5qSojGgaJpZM4JFSwW
.

@ktoso
Copy link
Member

ktoso commented Jul 5, 2016

Well, it's a step forward.

The reason it's in this main scope was basically because I hoped it would be easier to setup package tasks, which hasn't happened yet though

magro added a commit to magro/sbt-jmh that referenced this pull request Jul 5, 2016
ktoso pushed a commit that referenced this pull request Jul 5, 2016
@ktoso ktoso added this to the 0.2.8 milestone Jul 5, 2016
@richdougherty
Copy link

To get jmh:run to trigger a build I think you also need to add something like:

generateJmhSourcesAndResources in Jmh := ((generateJmhSourcesAndResources in Jmh) dependsOn(compile in Test)).value,

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.

None yet

3 participants