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 support for looking up optional pegdown dependency #11 #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leachbj
Copy link

@leachbj leachbj commented Nov 19, 2014

When using a recent version of scalatest the pegdown dependency
is required for htmlreports. To use htmlreports add the pegdown
dependency to the plugin definition in your project.

org.scalatest scalatest-maven-plugin org.pegdown pegdown 1.4.2

When using a recent version of scalatest the pegdown dependency
is required for htmlreports.  To use htmlreports add the pegdown
dependency to the plugin definition in your project.

  <plugin>
    <groupId>org.scalatest</groupId>
    <artifactId>scalatest-maven-plugin</artifactId>
    <dependencies>
      <dependency>
        <groupId>org.pegdown</groupId>
        <artifactId>pegdown</artifactId>
        <version>1.4.2</version>
      </dependency>
    </dependencies>
  </plugin>

private List<String> getPegdownDependencies() {
List<String> dependencies = new ArrayList<String>();
Artifact pegdown = pluginArtifactMap.get("org.pegdown:pegdown");

Choose a reason for hiding this comment

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

The original problem is only about pegdown dependency for reporting. I think it would be better to have this solution more generic - to work with any dependencies set on the plugin level.

@ziscloud
Copy link

Please merge these changes, really need it.

@bezda
Copy link

bezda commented Jan 17, 2016

Anyone still maintaining this plugin? This is pretty useful patch for anyone who can't have direct pegdown dependency in pom. +1 for merging even though it's not "generic enought"

@bvenners
Copy link
Contributor

Hi Tomas,

Yes, we're still here. We've just been overwhelmed trying to get the 3.0 release out the door, which primarily was adding Scala.js support to ScalaTest. Issues and PRs have gotten starved for attention all last year because of that. We are getting close to a 3.0 release, but still not there quite there. Need to figure out how to do timeouts in async styles, including time limited tests, and a bit more documentation. But squeaky wheels get grease. I'll assign someone this week to have a look at the Maven plugin issues and PRs.

Bill

@bvenners
Copy link
Contributor

Can you elaborate on what the problem is that this PR addresses? We didn't put a required dependency in ScalaTest on pegdown so that it wasn't needed if people didn't use HTML reports. If they want to use HTML reports, then we expected they will just add the pegdown dependency. Please first explain the pain point that you experience that the bit of code above is intended to address. Is it the pain of adding a dependency to the project instead of having it resolved automatically by the plugin?

@bezda
Copy link

bezda commented Jan 21, 2016

The problem with pegdown is that it brings multiple dependencies which are conflicting with our versions even if we specify it as test-only dependency and enforcer complains unless we add bunch of exclusions. Our projects don't really need pegdown for anything than reporting so we shouldn't be forced to add this dependency at project level. This pull request allows us to add pegdown as plugin dependency where it logically belongs

@bvenners
Copy link
Contributor

Thanks for clarifying the issue. I am thinking perhaps the best way to deal with the problem is to "shade" pegdown during our build. In other words, when we do a deploy, we grab the pegdown version we are depending on, change its package names to something under org.scalatest, and redeploy that as a separate jar that ScalaTest depends on. It is an extra jar for your build, but since it is just in test scope it shouldn't really matter. But you tell me. That way we could make it a required dependency, so it is easy for folks, or just leave it optional. In either case you shouldn't have any more troubles with version mismatch.

Does that sound like it would address the pain point you have?

Thanks.

Bill

(By the way, pegdown is released under the same Apache 2.0 license that ScalaTest is released under, which would allow this kind of "copying".)

@katrinsharp
Copy link
Collaborator

It's been sometime, so just wanted to see if it is still relevant. If it is, please resolve conflicts and tag me on the comment. Thank you.

@plinlor
Copy link

plinlor commented Aug 9, 2020

@cla-bot[bot] check

@cla-bot
Copy link

cla-bot bot commented Aug 9, 2020

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Bernard Leach.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email email@example.com
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Aug 9, 2020

The cla-bot has been summoned, and re-checked this pull request!

@cheeseng
Copy link
Contributor

Just fyi, I have submitted a PR to use commonmark which has 0 dependency:

scalatest/scalatest#2136

Hopefully it will be alternative solution for this.

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

8 participants