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

harmonize compliance profiles view with supermarket views #1654

Merged
merged 2 commits into from
Apr 13, 2017

Conversation

chris-rock
Copy link
Contributor

This PR:

  • updates the documentation
  • updates the inspec compliance profiles list to our supermarket view
  • allows compliance://admin/profile and admin/profile as input

screen shot 2017-04-12 at 11 04 53 pm

@chris-rock chris-rock force-pushed the chris-rock/optimize-compliance-cli branch from 77e21e2 to 29f46c0 Compare April 12, 2017 21:17
@chris-rock chris-rock changed the title harmonize profiles view with supermarket plugin harmonize compliance profiles view with supermarket views Apr 12, 2017
@chris-rock chris-rock force-pushed the chris-rock/optimize-compliance-cli branch from 29f46c0 to efd9153 Compare April 13, 2017 07:38
Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

LGTM. Just a question I'd love an answer to, and some suggestions for future. Nothing blocking except the failing travis test.

@@ -57,7 +57,14 @@ def login(server) # rubocop:disable Metrics/AbcSize
puts '', msg
end

desc "login_automate SERVER --user='USER' --ent='ENT' --dctoken or --usertoken='TOKEN'", 'Log in to an Automate SERVER'
desc "login_automate SERVER --insecure --user='USER' --ent='ENT' --usertoken='TOKEN'", 'Log in to an Automate SERVER'
Copy link
Contributor

Choose a reason for hiding this comment

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

I would love to see this be inspec compliance login --automate in the future than require a completely separate command. It feels weird to me from a CLI UX perspective. Not a blocker, obviously :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!

{ org: owner, name: name }
end
end.flatten
mapped_profiles = []
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use another Rubyism I love... #each_with_object:

mapped_profiles = profiles.values.each_with_object([]) do |org, memo|
  memo << org.values
end

Again, not a blocker, just passing along something I always try and address when I see an empty object being created just to populate it with enumeration.

@@ -260,6 +269,16 @@ def logout

private

# returns a parsed url for `admin/profile` or `compliance://admin/profile`
def sanitize_profile_name(profile)
if profile.is_a?(String) && URI(profile).scheme == 'compliance'
Copy link
Contributor

Choose a reason for hiding this comment

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

When would profile not be a String?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point :-)

@adamleff
Copy link
Contributor

@chris-rock Can we fix the failing functional test before merge?

Signed-off-by: Christoph Hartmann <chris@lollyrock.com>
Signed-off-by: Christoph Hartmann <chris@lollyrock.com>
@chris-rock chris-rock force-pushed the chris-rock/optimize-compliance-cli branch from efd9153 to a0da23f Compare April 13, 2017 15:14
@adamleff adamleff merged commit effd0dd into master Apr 13, 2017
@adamleff adamleff deleted the chris-rock/optimize-compliance-cli branch April 13, 2017 15:24
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

2 participants