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.

Sign up for GitHub

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

Jump to bottom

Basic Scalafiddle integration #16

Merged
merged 1 commit into from
Sep 5, 2016
Merged

Basic Scalafiddle integration #16

merged 1 commit into from
Sep 5, 2016

Conversation

andreaTP
Copy link
Contributor

Added a basic integration with ScalaFiddle.

@andreaTP
Copy link
Contributor Author

targeting #12 :-)
a bit of refactoring (probably need more at some point)

eed3si9n
eed3si9n reviewed Aug 25, 2016
View reviewed changes
core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala
@@ -1,12 +1,12 @@
/*
* Copyright © 2015 - 2016 Lightbend, Inc. <http://www.lightbend.com>
*
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this whitespace manually edited? It should be managed by sbt-header.

@eed3si9n
Copy link
Contributor

Could you add tests please?

@andreaTP
Copy link
Contributor Author

fixed space in header and added some basic test...
please let me know if you need more or a minimum coverage or whatever, I'm not confident that everything is stable and things will not change soon, so I start slow ...

b.t.w.
great tool! thanks!

pvlugter
pvlugter reviewed Aug 25, 2016
View reviewed changes
core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala
)
} catch {
case e: FileNotFoundException =>
throw new SnipDirective.LinkException(s"Unknown fiddle [${e.getMessage}] referenced from [${page.path}]")
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this should be FiddleDirective.LinkException.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true! thanks

@pvlugter
Copy link
Member

Cool. Nice feature.

@andreaTP
Copy link
Contributor Author

fix and rebased

@andreaTP
Copy link
Contributor Author

the failed test isn't related to this feature:

[info] - should parse and render all header children *** FAILED ***
[info]   "...h
[info]   <code>code</code>&[amp;]hellip;</h1>
[info]   
[info]   " did not equal "...h
[info]   <code>code</code>&[]hellip;</h1>
[info]   
[info]   " (AnchorLinkSpec.scala:29)

@andreaTP
Copy link
Contributor Author

andreaTP commented Sep 4, 2016

Anything yet I should do here?

@eed3si9n
Copy link
Contributor

eed3si9n commented Sep 5, 2016

Restarted the Travis job.

@eed3si9n
Copy link
Contributor

eed3si9n commented Sep 5, 2016

Merging because testing locally succeeded, but I'm guessing there's some race condition on the tests that we need to take care of eventually.

@eed3si9n eed3si9n merged commit 090a1e9 into lightbend:master Sep 5, 2016
@pvlugter
Copy link
Member

pvlugter commented Sep 5, 2016

Haven't seen test failures like this before either, locally or in CI. In the past it was running on Lightbend/Typesafe Jenkins builds, not Travis. Looks like it's always one of the http render checks, but not the same one?

@andreaTP
Copy link
Contributor Author

andreaTP commented Sep 6, 2016

thanks guys!

@andreaTP andreaTP mentioned this pull request Dec 8, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andreaTP @eed3si9n @pvlugter