Added @Language to String source code params #522
Conversation
| @@ -13,6 +13,7 @@ Spock is inspired from JUnit, jMock, RSpec, Groovy, Scala, Vulcans, and other fa | |||
| dependencies { | |||
| compile libs.groovy // easiest way to add Groovy dependency to POM | |||
| compile libs.junit | |||
| compile libs.annotations | |||
There was a problem hiding this comment.
shouldn't this be optional? You would need to add it to spock-specs as well, but I think this is better than to force it on users.
There was a problem hiding this comment.
Not sure what you mean by forcing it on the user (what drawbacks will the user have exactly? It would also eliminate the need for the custom @Nullable annotation already available) or how the used annotations could be optional (I don't think that if I put it in comments or put the dependency to runtime (i.e. non-packaged)) would work (however, I did find an update for the annotations on Maven Central)
There was a problem hiding this comment.
Just look two lines below compile libs.annotations, optional and I think it should be called intellijAnnotations.
Not sure what you mean by forcing it on the user
I mean we increase the amount of dependencies, even for users who use, e.g., eclipse and don't profit from it.
There was a problem hiding this comment.
The dependency only contains annotation declarations (http://grepcode.com/file/repo1.maven.org/maven2/org.jetbrains/annotations/13.0/org/intellij/lang/annotations), I think the added value supersedes the cost.
If you don't agree, I could still add only the Language.java file to the project (with the original package) and change the RetentionPolicy to SOURCE.
However, I think that's a hacky solution!
There was a problem hiding this comment.
Java ignores missing annotations so there shouldn't be any issues with
setting this dependency to optional. If someone has those annotations in his classpath it will they will be used, otherwise they'll be dropped.
https://stackoverflow.com/questions/3567413/why-doesnt-a-missing-annotation-cause-a-classnotfoundexception-at-runtime
There was a problem hiding this comment.
Ok, if you find it's better like that. Please see my updated commit (could you please retrigger the failing build).
|
I think that this is a valuable addition, as long as this can be an optional dependency. |
paplorinc
force-pushed
the
GroovyLanguageAnnotation
branch
4 times, most recently
from
a80cbc4
to
9bf6aae
Compare
paplorinc
force-pushed
the
GroovyLanguageAnnotation
branch
from
9bf6aae
to
5c3badb
Compare
paplorinc
force-pushed
the
GroovyLanguageAnnotation
branch
from
5c3badb
to
aa67e05
Compare
By annotating parameters with @language('Groovy') we'll have IDE support (e.g. syntax check and highlighting) for inlined languages (mostly used in tests)
paplorinc
force-pushed
the
GroovyLanguageAnnotation
branch
from
aa67e05
to
4eec054
Compare
Added @language to String source code params
By annotating parameters with @language('Groovy') we will have IDE support
(e.g. syntax check and highlighting) for inlined languages (mostly used in tests)