atlantis
atlantis copied to clipboard
runatlantis
fix: Use ISO 8601 date format
what
- Change output of timestamps to be ISO 8601 standard format (YYYY-MM-DD) instead of (DD-MM-YYYY)
why
- DD-MM-YYYY is confusing to people who are used to MM-DD-YYYY and it doesn't sort correctly.
tests
- It's a simple change and I don't have time to run tests
references
people rely on this since this is standard for a while in atlantis, we just can't change it because is confusing for some people.
@jamengual what if it was behind a flag?
I would be fine with that. I looked for a config flag at first and didn't find one. I don't quite know how to do that. I don't know golang unfortunately.
yeah, it should be behind a flag, and we can deprecate/remove the old timestamp format.
we are trying to not add new flags so if this leaves on the server side config might be better
On Thu, Jan 11, 2024, 5:42 p.m. Rui Chen @.***> wrote:
yeah, it should be behind a flag, and we can deprecate/remove the old timestamp format.
— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/pull/4141#issuecomment-1887931428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERBXKJTI5YLUUEHGE23YOBFE5AVCNFSM6AAAAABBWZDYTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBXHEZTCNBSHA . You are receiving this because you were mentioned.Message ID: @.***>
What's so bad about adding more flags?
Is the fear that we'll add a flag and not deprecate it later when it's chosen as the new path forward (breaking change) ?
If so, then I'd suggest we add a flag and time-box it. If the flag is opt-in and it doesn't cause issues and it's better practice, then in a follow up release, we should remove the flag and set the feature as the default.
What do you folks think?
What's so bad about adding more flags?
The more flags we add the harder it is for users to know what combination they need, the longer and more confusion our documentation becomes, and the larger the space of possible configuration combinations, many of which are untested.
Additionally, adding flags means we have to support that behavior unless we explicitly deprecate it, which takes a lot of time and effort. I recently put several hours into #2992, which of course is necessary for any large project, but adds up. There's very little pressure on removing flags/features, so it's very easy for them to grow without bound.
Certainly configurability has benefits, and any one option (including this one) might be perfectly reasonable, I'm just trying to call out the tradeoff, especially cumulatively.
For what it's worth, in this case I would argue to unconditionally change to YYYY-MM-DD. It conforms more to standards and is less ambiguous.
As for it affecting users, if I expected "17-01-2023" and saw "2023-01-17", even if I really preferred "17-01-2023", I feel like I would understand quickly what happened. Even if it were being parsed by some code (hopefully not!), the failure would be most likely obvious. If we were going from one arbitrary format to another I would be more sympathetic, but we're going towards a more standard ones, and I'd imagine our users would sympathize.
(As an aside, I'm actually not sure if I've ever seen DD-MM-YYYY anywhere before. Certainly DD/MM/YYYY or MM/DD/YYYY, but with the dashes it looks very strange to me)
it is true that it could be a trivial change.
then let's document it in the doc and not add a flag for it and release a new minor version.
On Wed, Jan 17, 2024, 8:44 p.m. Luke Massa @.***> wrote:
For what it's worth, in this case I would argue to unconditionally change to YYYY-MM-DD. It conforms more to standards and is less ambiguous.
As for it breaking users, anyone/any process who depends on MM-DD-YYYY will fail to parse, and the fix on their end should be straightforward. If we were going from one arbitrary format to another I would be more sympathetic, but we're going towards a more standard ones, and I'd imagine our users would sympathize.
— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/pull/4141#issuecomment-1897787886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERBMZ7HASIHTMNJQN3TYPCSBTAVCNFSM6AAAAABBWZDYTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJXG44DOOBYGY . You are receiving this because you were mentioned.Message ID: @.***>
@jdmulloy I took the liberty of fixing the failed unit test, very straightforward
I agree with @lukemassa; we should add flags for something like that and conform to standards. In this case, we can change and highlight a breaking change as part of a minor release.
another way of thinking about the flags, we can RFC introducing some prefix (serving some flag grouping purpose so that we can prioritize the deletions when the migration is done. )