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

Add @@dependency directive #142

Merged
merged 2 commits into from
Sep 11, 2017
Merged

Conversation

jonas
Copy link
Contributor

@jonas jonas commented Sep 1, 2017

Shows tabbed build tool snippets for using a JVM library.


Alpakka has quite a lot of these and Akka HTTP is starting to as well so it might be good to have a special directive for rendering this.

screen shot 2017-09-01 at 11 11 57 am

@jonas
Copy link
Contributor Author

jonas commented Sep 1, 2017

Akka HTTP also uses cross CrossVersion.full, so let me know if it would make sense to try to do this as part of the scala version suffix parsing or by having a sbtCrossVersion option.

Shows tabbed build tool snippets for using a JVM library.
@jonas jonas force-pushed the dependency-directive branch from 8ffc02f to 2736a34 Compare September 4, 2017 02:29
Copy link
Contributor

@2m 2m left a comment

Choose a reason for hiding this comment

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

Looks very good! Where did you come across CrossVersion usage in Akka Http docs?

@jonas
Copy link
Contributor Author

jonas commented Sep 4, 2017

Several places when referring to the Akka HTTP artifacts. I guess it is for when it's compiled against a Scala RC version. Would it make sense to try to match Scala RC versions in the artifact name like here?

@2m
Copy link
Contributor

2m commented Sep 5, 2017 via email

@2m
Copy link
Contributor

2m commented Sep 8, 2017

I am curious about the default case:

@@dependency[sbt, Maven]{
  group=com.typesafe
  artifact=config
  version=1.3.1
}

For this example the sbt output would contain a single % thus with no cross-scala version support. Which is fine for this particular library, but it is not the most common case (scala libs).

What do you think if we would have %% in sbt and _${scala.binary} in others as a default? This would cover most of the Scala libraries. And for java libs we would need to have something special that would indicate that no cross scala version needs to be specified.

@jonas
Copy link
Contributor Author

jonas commented Sep 8, 2017

I think it will be too error prone if we default to append the Scala binary version to the artifact name. In addition to having something special for java libs you'd also need something special when artifacts are versioned with CrossVersion.full.

For Scala dependencies this is pretty clear, yes it adds a bit of noise for the artifact, but it has less chance of doing the wrong thing:

@@dependency[sbt, Maven]{
  group=com.typesafe.akka
  artifact=akka-http_$scala.binary.version$
  version=$project.version$
}

The sbt snippet will show it as "com.typesafe.akka" %% "akka-http" % "10.0.10"

@2m
Copy link
Contributor

2m commented Sep 8, 2017

Great! I did not realise that we will be able to use $...$ placeholders in the directive itself. Great stuff!

@ktoso
Copy link
Contributor

ktoso commented Sep 11, 2017

This looks great, LGTM; One problematic thing is that sometimes we need to show multiple dependencies and would be nice to do so in the same snippet. Perhaps that could be a new directive that would be "@@dependencies" I guess though

@ktoso ktoso merged commit dafa8d9 into lightbend:master Sep 11, 2017
@ktoso ktoso modified the milestones: 0.2.13, 0.2.15 Sep 11, 2017
@jonas jonas deleted the dependency-directive branch September 11, 2017 13:31
@jonas
Copy link
Contributor Author

jonas commented Sep 11, 2017

Yes, in Akka HTTP there's one such case, I believe. @ktoso do you have any suggestions on how the syntax should be? I am not sure if the directive syntax parsing can be nested.

@2m 2m modified the milestones: 0.2.15, 0.3.0 Feb 9, 2018
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