You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2018/11/16 21:06:13 UTC

[GitHub] rob05c commented on issue #3007: Change Monitor stat history to sync.Map

rob05c commented on issue #3007: Change Monitor stat history to sync.Map
URL: https://github.com/apache/trafficcontrol/pull/3007#issuecomment-439527202
 
 
   Can you give me an idea of what you want to see here? This is really just changing a mutexed map, to a sync.Map. I'm having a hard time coming up with a good test, for the changes this makes. I mean, I could write a test to verify the data put in the map is what we get out; but testing the Go Standard Library doesn't seem that valuable.
   
   I agree, the `CreateStats` function that this is used by at a high level would benefit from a test. But it's a pretty big function, arguably about 1/3 of the overall logic in the Monitor, likely several weeks to write. I'd certainly support prioritizing and writing that. But in the context of this PR, several weeks of tests, for several days of code?
   
   I'm having a hard time coming up with a useful test that's any smaller.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services