airspeed-velocity / asv

Airspeed Velocity: A simple Python benchmarking tool with web-based reporting
https://asv.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
862 stars 180 forks source link

JSONC support is bugged #1425

Closed lucascolley closed 4 weeks ago

lucascolley commented 1 month ago

@HaoZeke there was an oversight in my PR - the default is actually defined at the level of the command as well as in config.py:

https://github.com/airspeed-velocity/asv/blob/bc2d07e8fab8926706e9d8ee7bf506dea6479b41/asv/commands/common_args.py#L26-L29

I think we should make it so that the command defaults to None, and let config.py handle the None?

HaoZeke commented 1 month ago

@HaoZeke there was an oversight in my PR - the default is actually defined at the level of the command as well as in config.py:

https://github.com/airspeed-velocity/asv/blob/bc2d07e8fab8926706e9d8ee7bf506dea6479b41/asv/commands/common_args.py#L26-L29

I think we should make it so that the command defaults to None, and let config.py handle the None?

Sure that makes sense. Sorry I didn't catch it in the review.