Skip to content

Conversation

@raboof
Copy link
Member

@raboof raboof commented Nov 10, 2025

Instead of having 2050-01-01 as 'magic no expiration date', just leave this field out.

@raboof raboof marked this pull request as draft November 10, 2025 08:58
@raboof raboof force-pushed the simplify-nonexpiration branch from 1b2ccf3 to cacb617 Compare November 10, 2025 09:01
Instead of having 2050-01-01 as 'magic no expiration date',
just leave this field out.
@raboof raboof force-pushed the simplify-nonexpiration branch from cacb617 to 3d7ed77 Compare November 10, 2025 09:03
@raboof raboof marked this pull request as ready for review November 10, 2025 09:05
@raboof raboof mentioned this pull request Nov 10, 2025
6 tasks
for name, refs in actions.items()
for ref, details in refs.items()
if date.today() < details.get("expires_at") or details.get("keep")
if (not details or "expires_at" not in details or date.today() < details["expires_at"]) or details.get("keep")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my personal Python education...
Can details actually be None? (similar in L151 + L162)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it looks like the yaml parser we're using turns a key without any children into None instead of an empty dict. I guess that makes sense since at the yaml level it's not possible to distinguish between "a key without a value" and "a key without children". To make this nicer we'd have to do some kind of 'model-based yaml parsing', which might not be worth it.


import ruyaml

indefinitely = date(2050, 1, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@snazy snazy mentioned this pull request Nov 10, 2025
Copy link
Member

@snazy snazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for the clarification!
+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants