Order metadata by groupId

tumblr / collins

groovy kind of love

tumblr.github.com/collins

Apache License 2.0

572 stars 99 forks source link

Order metadata by groupId #535

Open michaeljs1990 opened 7 years ago

michaeljs1990 commented 7 years ago

This orders metadata by the groupId of that metadata before passing it onto other functions for processing. Since a Map was previously used you could end up with your data being rendered in any which order. Now data provided you map it correctly in the following functions will be displayed in the proper order.

TODO:

[ ] write some tests that verify all reconstructing* functions do this properly.

Report: https://github.com/tumblr/collins/issues/534

michaeljs1990 commented 7 years ago

All tests still pass but someone else should look at this as i'm not sure if it could have other subtle effects.

byxorna commented 7 years ago

This LGTM, but I would definitely like unit tests to ensure expected behavior. Can you contrive a fixture lshw/lldp that illustrates the behavior?

byxorna commented 7 years ago

@michaeljs1990 bump, checking on unit tests. Is this merge blocking anything internal for you?

michaeljs1990 commented 7 years ago

No this one is not. I was actually just bored that night so I tried to fix the issue in #534 since it sounded interesting. I'll likely get some unit tests added for this over the weekend.