-
Notifications
You must be signed in to change notification settings - Fork 134
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
Remove packages with known issues on debian/ubuntu #93
Conversation
It looks like there is a problem with the ruby gems/dependencies for the Ruby 1.9.3 CI job:
There are also problems with coveralls on all the other CI jobs:
|
awesome @mikemoate #96 is merged by now, could you rebase? We fixed the tests. |
Thanks @chris-rock I have rebased and the CI build is in progress. I have a further change ready to expose the list of packages to clean as an attribute (which users can then override to include more packages or exclude any of the defaults if needed), whilst preserving the default list and behaviour. Should I add that commit to this pull request, or would you rather consider it separately? |
@chris-rock What is needed to progress this pull request? We're keen to contribute back to the project rather than maintain a fork. |
I just gave everything a once over and its inline with the REL feature and looks very well laid out. My only gripe is a lack of testing. Still +1 👍 |
# Cookbook Name:: os-hardening | ||
# Library:: apt_package_extras | ||
# | ||
# Copyright 2015, Hardening Framework Team |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add the original authors as well?
Looks great to me. Once we added the required information for the license, I am happy to merge |
@chris-rock apologies for that, I've now added the Opscode copyright from https://github.com/chef/chef/blob/master/lib/chef/provider/package/apt.rb to this file. Let me know if that's not what you meant/intended. |
@mikemoate No issue, the license requires to add the header. So we are good now? Could you do me one last favor and rebase it to the latest master? |
This was complicated by the existence of virtual packages in the debian distro family. A library with helper functions for this was addded. HardeningFramework-DCO-1.1-Signed-off-by: Mike Moate <[email protected]> (github: mikemoate)
My regex skills are not strong enough to rewrite this line yet. Disable rubocop warning for now to get a CI build to complete. HardeningFramework-DCO-1.1-Signed-off-by: Mike Moate <[email protected]> (github: mikemoate)
Add the Opscode copyright from https://github.com/chef/chef/blob/master/lib/chef/provider/package/apt.rb since portions are based on this code.
Now rebased and CI is clean. |
Great work @mikemoate ! |
Remove packages with known issues on debian/ubuntu
This is to resolve #90
There are no existing tests for this functionality on the redhat distro family, I haven't added any for the debian family, though in theory a generic test could now be possible.
There is one remaining rubocop warning relating to use of '$/', however this is code taken from the Chef apt package provider (to have consistent behaviour) so I am reluctant to alter this (my regex skills are too weak).
I'd like to make the list of packages to remove an attribute, with the current list as the default. Would that also be an acceptable change? I can add that to to this pull request or pick up separately?