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

extract class dependency from 'classOf' Literal #510

Merged
merged 2 commits into from Mar 13, 2018

Conversation

natansil
Copy link
Contributor

@natansil natansil commented Mar 8, 2018

fixes issue #508

@lightbend-cla-validator
Copy link

Hi @natansil,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@natansil
Copy link
Contributor Author

natansil commented Mar 8, 2018

@jvican as requested

@natansil
Copy link
Contributor Author

natansil commented Mar 8, 2018

I've signed the CLA

@jvican
Copy link
Member

jvican commented Mar 8, 2018

@dwijnand @eed3si9n CLA checking seems not to be working in sbt-whitesource.

Copy link
Member

@jvican jvican left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR 😄 Just one comment.

@@ -112,6 +112,25 @@ class DependencySpecification extends UnitSpec {
assert(inheritance("B") === Set.empty)
}

it should "extract class dependency from 'classOf' literal" in {
Copy link
Member

Choose a reason for hiding this comment

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

Could you also add a scripted (integration) test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean adding a test a long side this one?

Copy link
Member

Choose a reason for hiding this comment

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

I mean adding a test like that one but that would fail compilation if it this fix wasn’t present, and that would succeed after the patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dwijnand
Copy link
Member

dwijnand commented Mar 9, 2018

ci is failing because of a typo in #496. fixed in #511

@dwijnand dwijnand added this to the 1.2.0 milestone Mar 13, 2018
@dwijnand dwijnand merged commit d6dec01 into sbt:1.x Mar 13, 2018
@dwijnand
Copy link
Member

thank you @natansil!

@natansil
Copy link
Contributor Author

@dwijnand Happy to contribute.

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

Successfully merging this pull request may close these issues.

None yet

4 participants