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

Fix scoping in RPM plugin for #789 #826

Merged
merged 3 commits into from Aug 5, 2016
Merged

Conversation

thetristan
Copy link
Contributor

@thetristan thetristan commented Jun 26, 2016

This change updates the RPM plugin settings so that keys starting with rpm are no longer scoped to the Rpm configuration. This should address #789.

I didn't see a formal testing process documented (though I may have missed it in the docs), so far I've only verified that sbt test and sbt scripted rpm/* are passing (both ran on a VM running centos7).

TODO

  • Investigate if additional tests are needed, and if so add them

@muuki88 muuki88 added the rpm label Jun 26, 2016
@muuki88
Copy link
Contributor

muuki88 commented Jun 26, 2016

Thanks for your pull request. Code LGTM. The is a minimal Contributing.md, which describes how to run the tests. We don't have a"formal test process", as the test-matrix for this project is almost infinite :(

There is a simple test sbt-test/rpm/simple-rpm, which only checks if the rpm has been built. It would be nice to extend this test to check if the spec file contains the correct contents. The sbt-test/rpm/systemd-rpm build.sbt has an example on how to check the spec file.

@muuki88
Copy link
Contributor

muuki88 commented Jul 1, 2016

@thetristan do you need any help for extending the tests?

@thetristan
Copy link
Contributor Author

@muuki88 I should be OK. I've been busy with real life kind of things but I'll try to get this change wrapped up this weekend.

@thetristan
Copy link
Contributor Author

I updated the base test for simple-rpm to check the values for the existing keys in that project. I'll extend the test at some point later today to add coverage for the additional keys that aren't prefixed with rpm*, too.


TaskKey[Unit]("checkSpecFile") <<= (target, streams) map { (target, out) =>
val spec = IO.read(target / "rpm" / "SPECS" / "rpm-test.spec")
println(spec)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see traces of debugging sbt :D

@muuki88
Copy link
Contributor

muuki88 commented Jul 25, 2016

looks good so far :)

@thetristan
Copy link
Contributor Author

OK, I was finally able to add those additional checks. Let me know if you see anything else that should be improved.

@thetristan
Copy link
Contributor Author

At first glance it looks like the build failure from my last push is unrelated. I rebased my commits to re-trigger the build and will investigate further if it fails again.

@muuki88 muuki88 merged commit d474fd7 into sbt:master Aug 5, 2016
@muuki88
Copy link
Contributor

muuki88 commented Aug 5, 2016

Thanks a lot. Will release this ASAP.

@thetristan
Copy link
Contributor Author

Sure, you're welcome!

@thetristan thetristan deleted the fix-rpm-scoping branch August 5, 2016 19:45
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.

None yet

2 participants