Skip to content

Conversation

movitto
Copy link
Contributor

@movitto movitto commented Oct 31, 2013

Native package system lookup method for rpm and deb
package systems are provided as well as an abstract
mechanism so the user doesn't have to know distro details

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling d887a9b on movitto:get-pkg-info into 614e93d on ManageIQ:master.

Copy link
Member

Choose a reason for hiding this comment

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

prefer each_with_object over inject for modifying hashes like this so you don't have to care about returning the object at the end of the block.

Copy link
Member

Choose a reason for hiding this comment

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

I also suggest you process the line(s) in a method so you can write tests with lots of different input without having to call/stub apt-cache.

@movitto
Copy link
Contributor Author

movitto commented Nov 6, 2013

@jrafanie @Fryguy updated, here are my thoughts:

  • the return value of inject is what is actually returned from the method so it is needed in those cases
  • tried splitting the per-line processing as we did w/ fstab but it's tricky in this case since most of the tag processing is to maintain an in_description state that spans across multiple lines. Did my best to try to split it up while keeping it clean / simple
  • Rpm and Deb have been changed to subclasses of Package. The delegator method is needed incase the user does not know or does not want to have to deal w/ the Distro details before delegating to the native package call. The patch I submitted to bundler_ext (see the referenced PR above) relies on this so that bundler_ext can extract native package system info w/out worrying about the specifics.

Shout out if anything else looks off / needs to be updated to make this in.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling 01f063d on movitto:get-pkg-info into 614e93d on ManageIQ:master.

@Fryguy
Copy link
Member

Fryguy commented Nov 6, 2013

the return value of inject is what is actually returned from the method so it is needed in those cases

each.with_object also returns the accumulated object

Native package system lookup method for rpm and deb
package systems are provided as well as an abstract
mechanism so the user doesn't have to know distro details.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling 541b185 on movitto:get-pkg-info into 614e93d on ManageIQ:master.

@jrafanie
Copy link
Member

jrafanie commented Nov 8, 2013

All good to me 👍. @Fryguy Did you have anything else before this is merged?

@jrafanie
Copy link
Member

jrafanie commented Nov 8, 2013

@Fryguy indicates we're all good.

jrafanie added a commit that referenced this pull request Nov 8, 2013
Add mechanism to lookup specific package information
@jrafanie jrafanie merged commit 3502a00 into ManageIQ:master Nov 8, 2013
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.

4 participants