[GitHub] [trafficserver] maskit commented on pull request #9485: Unit parser

["maskit (via GitHub)" <gi...@apache.org>]

maskit commented on PR #9485:
URL: https://github.com/apache/trafficserver/pull/9485#issuecomment-1452590914

   Having this parser is fine. It'd make sense for file/memory size, but I'm skeptical to use it for amount of time such as timeout.
   
   `abc.max_buf_size: 10M` -- Makes sense, it might hit internal upper limit but I don't think there'd be confusion.
   `xyz.timeout: 100ms` -- Even if there's no upper nor lower limit, It's not obvious whether milliseconds makes sense.
   
   We currently use second for settings that receive amount of time in most of cases. And if a setting expects millisecond, the setting name has `_ms` suffix. It's obvious that the percision is millisecond (if not, the name should not have _ms and just use second like others).
   
   It's not only about percision. The suffix roughly suggests reasonable amount of time. Setting 100000 to a setting that has _ms suffix looks odd.
   
   Of course we could describe things on the documentation and say RTFM, but I don't think providing super detailed 1000 pages of documentation is user friendly.
   


-- 
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: github-unsubscribe@trafficserver.apache.org

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