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

Denylist PlayWeb configuration names with undefined config keys. Fixes #57 #58

Merged
merged 1 commit into from
Aug 23, 2019
Merged

Denylist PlayWeb configuration names with undefined config keys. Fixes #57 #58

merged 1 commit into from
Aug 23, 2019

Conversation

matmannion
Copy link
Contributor

@matmannion matmannion commented Aug 6, 2019

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

What does this PR do?

Adds three new configuration keys, web-assets, web-plugin and web-assets-test to the denylist of config keys that are used in the Snyk sbt plugin (currently opt-in with --sbt-graph) to enable the plugin to work out of the box with Play Framework projects that use the standard Play plugin.

How should this be manually tested?

As per the instructions in #57:

  • Create a new Play scala seed project: sbt new playframework/play-scala-seed.g8
  • Accept the defaults; cd play-scala-seed
  • Copy the Snyk sbt plugin from this PR to project/SnykSbtPlugin.scala
  • Install the sbt-dependency-graph plugin locally on the project: echo 'addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.10.0-RC1")' | tee --append project/plugins.sbt
  • Run sbt snykRenderTree

What are the relevant tickets?

Fixes #57

@lili2311
Copy link
Contributor

lili2311 commented Aug 8, 2019

👋 Thank you for this awesome contribution, would you be able to add a test for this please to show the issue and that the repo now is successfully tested? Sounds like we can add aplayframework/play-scala-seed.g8 to fixtures and have a simple test to verify it scanned OK. Please let me know if you have any questions.

@CLAassistant
Copy link

CLAassistant commented Aug 13, 2019

CLA assistant check
All committers have signed the CLA.

@matmannion
Copy link
Contributor Author

Hi @lili2311, I've added a system test with play-scala-seed using --sbt-graph, and making sure that the version parses out correctly.

@lili2311
Copy link
Contributor

Thanks again for this contribution ❤️ It looks almost ready to merge.
Could you please update your commits to use feat: and maybe squash into 1 commit? Otherwise this won't release as we need semantic release style commit titles.

@matmannion
Copy link
Contributor Author

That's squashed into a single feat: commit now.

@lili2311 lili2311 self-assigned this Aug 23, 2019
Copy link
Contributor

@lili2311 lili2311 left a comment

Choose a reason for hiding this comment

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

🎉

@lili2311 lili2311 merged commit 66ce80e into snyk:master Aug 23, 2019
@lili2311 lili2311 mentioned this pull request Sep 2, 2019
3 tasks
@snyksec
Copy link

snyksec commented Sep 2, 2019

🎉 This PR is included in version 2.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

sbt-graph option doesn't work out of the box on Play Framework webapp projects
4 participants