search

canonical / pebble

Take control of your internal daemons!
https://canonical-pebble.readthedocs-hosted.com/
GNU General Public License v3.0
143 stars 54 forks source link

fix(timeutil): remove unreachable panic() call #455

Open thp-canonical opened 1 month ago

thp-canonical commented 1 month ago

This has been added in upstream snapd back in the day here:

Move conditionals to the correct case where they belong (e.g. >= -cutoffDays belongs to the "%d days ago", ...), and then just naturally move the non-special case of then.Format("2006-01-02") to the default case.

Looking at compiler explorer, it seems like Go (gc 1.21 on x86_64) can optimize out the old version's panic(), so it's just dead code in the source (as it should be): https://godbolt.org/z/fxqTzYfPv

benhoyt commented 1 month ago

Does this actually cause any problems? I'm not opposed to this, though might be better to get it into snapd first? Otherwise it seems to me "if it ain't broke, don't fix it".

thp-canonical commented 1 week ago

Proposed fix in snapd: https://github.com/canonical/snapd/pull/14427