Skip to content
This repository has been archived by the owner on Aug 28, 2021. It is now read-only.

newReport functionality #3

Open
virtuallynathan opened this issue Feb 24, 2016 · 3 comments
Open

newReport functionality #3

virtuallynathan opened this issue Feb 24, 2016 · 3 comments

Comments

@virtuallynathan
Copy link
Contributor

I was wondering if there was a reason newReport returned a map[string][]int, which requires sprintf'ing the integer version of the map into the string-based map?

It seems like most of the functions take in sent, received, and paths. If the report struct is changed to integer keyed maps, many functions could take in that struct instead.

I started making these changes here: virtuallynathan@137370e
Stuff still seems to work as intended, but I don't have a large ECMP fabric to test against.

@coxley
Copy link
Contributor

coxley commented Nov 8, 2016

I lack the specific context for why, but at the surface it sounds more idiomatic to do the way you say yeah.

@virtuallynathan
Copy link
Contributor Author

virtuallynathan commented Nov 8, 2016

Is this a change worth making? If so, I'll see if I can rebase my changes on the latest commits here and submit a PR.

@coxley
Copy link
Contributor

coxley commented Nov 8, 2016

I'd accept a diff changing the relevant functions to accept Report instead, yeah. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants