ManageIQ / polisher

Polisher is a Ruby module and set of utilities aimed to assisting the post-publishing packaging process for Ruby gems and applications.
MIT License
14 stars 14 forks source link

[WIP/POC] Koji diff enhancements #116

Closed jrafanie closed 9 years ago

jrafanie commented 10 years ago

I really don't like the names of the methods or the interfaces of the methods, this is really to get an idea of what others think. I'm hoping the specs help illustrate the use cases of wanting to be able to see what rpm versions were added, removed, changed, or unchanged between two koji tags.

jrafanie commented 10 years ago

cc @brandondunne @movitto Not ready for merge, still really WIP... What are your thoughts?

I decided to not spend much time on naming methods or extracting a Tag or Comparison class to house these methods... yet.

jrafanie commented 10 years ago

cc @strzibny @axilleas do you have thoughts on use cases and design of this feature?

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.13%) when pulling 7f9306b3e214a378327514c1aa540ea01131949e on jrafanie:koji_diff_enhancements into 48990f5eac37d28c2dfb5da59297fae9181d4a05 on ManageIQ:master.

miq-bot commented 10 years ago

Checked commits https://github.com/jrafanie/polisher/commit/166e9e4b9f2fda80c3c190ce7d30ad177797f28d .. https://github.com/jrafanie/polisher/commit/7f9306b3e214a378327514c1aa540ea01131949e with rubocop 0.21.0 2 files checked, 8 offenses detected

spec/koji_spec.rb

bdunne commented 10 years ago

Looks ok, I can't think of a better way to select the data that you want to report right now. "207,408 additions and 12 deletions." <-- wasn't expecting that from you :astonished:

bdunne commented 10 years ago

@jrafanie Are all of the VCR casettes are the same? If so, should they share one YAML file to save space / update headache?

strzibny commented 10 years ago

Sometimes we do builds in a side-tag for new ruby/rubygems etc. so the use case could be simply to have a list of rebuilt and not-yet rebuilt RPMs compared to master. Is this what is suppose to handle?

movitto commented 10 years ago

@jrafanie looks good. Assuming that since you want to control build inheritance on a per tag basis the other methods should have 'options' parameters? (minor nit but could you change the comment width in the 'diff' method so that the column names still line up with the client.call params)

Perhaps a single method #tagged_diff could be implemented that returns a hash containing the :additions, :removals, :overlapping, :unchanged. Would be able to leverage a single call to 'diff' which should help with perf. Up to you.

@strzibny this could be used for that use case + more. Essentially if you have any two koji tags this can compare gems / gemsets between them.

jrafanie commented 10 years ago

@brandondunne Yeah, sounds good. I'll definitely clean up the VCR cassettes to only use what we need.

@movitto Thanks. yeah, I agree on your points.

@strzibny I guess the idea is we have several differences we can present and I agree we can title them as keys in a hash or something like that.

Here's the possibilities:

1) In tag 1 and tag 2, same version (what I called overlapped unchanged)
2) In tag 1 and tag 2, but different version (overlapped changed)
3) In tag 1, not tag 2 (removals)
4) In tag 2, not tag 1 (additions)

For @strzibny's use case of a list of rebuilt vs. not-yet rebuilt rpms... does any of the below describe what could be used in your process: In both side-tag and "master" tag (same version) (same as 1) above In both side-tag and "master" tag (different version) (same as 2) above In master tag, not side-tag ( either 3 or 4 above depending which tag is your reference point)

Are there other scenarios I'm missing? Can someone suggest better names? Additions/removals carries context (which tag we're referring to)... do terms like intersection/union make more sense?

I can't come up with better names... maybe something like this:

1) In tag 1 and tag 2, same version, (common tagged builds)
2) In tag 1 and tag 2, but different version (common packages with different versions)
3) In tag 1, not tag 2 (packages unique to tag1)
4) In tag 2, not tag 1 (packages unique to tag2)
jrafanie commented 10 years ago

@movitto RE: Assuming that since you want to control build inheritance on a per tag basis the other methods should have 'options' parameters?

Yeah, it's an ugly interface right now. I imagine you might want to use tag inheritance in one or both tags you're comparing. The interface would be a bit cleaner if we decide on using a method like you describe, where it runs the diff once and returns a hash with the keys: :additions, :removals, :overlapping, :unchanged.

I can't get past the names right now. I guess I can clean this up as suggested above and see what it looks like but I still don't like the names, especially additions and removals since it depends what tag is used a reference point.

jrafanie commented 9 years ago

Stale, closing