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 2022/04/14 16:21:10 UTC

[GitHub] [trafficcontrol] rob05c commented on pull request #6673: if no parents exist t3c will use secondary parents if configured

rob05c commented on PR #6673:
URL: https://github.com/apache/trafficcontrol/pull/6673#issuecomment-1099365975

   ` This PR has tests` `This PR has documentation` `This PR has a CHANGELOG.md entry` are all checked, but it doesn't look like any of those are in the PR. @jpappa200 mind adding them?
   
   I don't think we need docs, this behavior is incidental and the only right thing to do, we don't need to document that level of detail. But that should probably be noted, rather than checking the box that it has docs when it doesn't.
   
   But it should probably have a Changelog entry, so people know how and when it changed.
   
   And it should be straightforward to add a unit test, shouldn't need an Integration Test. In https://github.com/apache/trafficcontrol/blob/3b8c0cdc/lib/go-atscfg/parentdotconfig_test.go just needs to make a test with no Primaries and a Secondary via CacheGroups, which should pass with the code change and fail without it.
   
   Other than that, this change looks good. The previous behavior was generating broken config, this is the right thing to do.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org