Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Basic infrastructure for veryfing code samples #946

Merged
merged 12 commits into from Apr 12, 2013

Conversation

Projects
None yet
4 participants
Owner

jroper commented Apr 4, 2013

  • documentation is now a project that gets tested as part of run-tests
  • Extended markdown syntax to supporting pulling in code samples from
    external files
  • Migrated ScalaActions.md to use this

@huntc huntc commented on the diff Apr 4, 2013

framework/project/Dependencies.scala
@@ -137,7 +137,7 @@ object Dependencies {
"com.h2database" % "h2" % "1.3.168",
"org.javassist" % "javassist" % "3.16.1-GA",
- "org.pegdown" % "pegdown" % "1.1.0",
+ "org.pegdown" % "pegdown" % "1.2.1-withplugins-1",
@huntc

huntc Apr 4, 2013

Collaborator

Shouldn't that be a snapshot release? I've now enhanced the build via #945 so that typesafe snapshots are considered when building snapshot versions of Play.

@jroper

jroper Apr 4, 2013

Owner

Maybe... but the fact is we need to (for now) be on a fork, and the fork works so I cut a release of it.

@huntc

huntc Apr 4, 2013

Collaborator

I love the confidence ;-)

@jroper

jroper Apr 4, 2013

Owner

Confidence would be cutting 1.2.1-withplugins, the -1 implies there will likely be more to come :)

@richdougherty

richdougherty Apr 8, 2013

Owner

I'd consider putting the fork inside com.typesafe.pegdown rather than discriminating between releases by adding withplugins to the version.

@jroper

jroper Apr 9, 2013

Owner

The difficulty with changing the groupId is that if we do that, we should really also change the package names, because two different groupIds means you can very easily get two different artifacts on the classpath with the same class files in them, and then it's russian roulette as to whether it works or not.. By discriminating using the version number, then we can be sure that there is only one version of the classes on the classpath, and it's easy to determine exactly which version, and easy to fix (simply add an explicit dependency to that version in your build, rather than potentially adding many excludes to all the artifacts that try to bring in the wrong groupId artifact).

@richdougherty

richdougherty Apr 12, 2013

Owner

OK that makes sense!

play2-master #998 SUCCESS
This pull request looks good

play2-master-PRs #1 SUCCESS
This pull request looks good

@richdougherty richdougherty commented on an outdated diff Apr 9, 2013

...in/src/main/scala/play/markdown/MarkdownSupport.scala
+
+ // Label is after the #, or if no #, then is the link label
+ val (source, label) = code.getSource.split("#", 2) match {
+ case Array(source, label) => (source, label)
+ case Array(source) => (source, code.getLabel)
+ }
+
+ // The file is either relative to current page page or absolute, under the root
+ val sourceFile = if (source.startsWith("/")) {
+ new File(root, source.drop(1))
+ } else {
+ new File(new File(root, pagePath), source)
+ }
+
+ if (!sourceFile.exists()) {
+ throw new IllegalArgumentException("Could not find source file: " + sourceFile)
@richdougherty

richdougherty Apr 9, 2013

Owner

Not a big deal, but usually a missing file is an IOException.

play2-master-PRs #23 FAILURE
Looks like there's a problem with this pull request

play2-master-PRs #35 SUCCESS
This pull request looks good

huntc added a commit that referenced this pull request Apr 12, 2013

Merge pull request #946 from jroper/code-samples
Basic infrastructure for veryfing code samples - awesome work!

@huntc huntc merged commit 289ca2c into playframework:master Apr 12, 2013

Collaborator

huntc commented Apr 12, 2013

Awesome.

@jroper jroper deleted the jroper:code-samples branch Apr 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment