Skip to content

Conversation

@mnencia
Copy link
Member

@mnencia mnencia commented Dec 3, 2024

Closes #48

jsilvela
jsilvela previously approved these changes Dec 4, 2024
Copy link

@jsilvela jsilvela left a comment

Choose a reason for hiding this comment

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

Looks good.
One comment is that we could have a begin time in ISO format, and an end time in non-ISO format, and the code would work fine.
Would it make sense to add a consistency check? Either we use ISO format for both begin/end, or for neither.
In any case, optional.

    if b.EndTimeISOString != "" {
         ... snipped ...
    } else if b.EndTimeString != "" {

@mnencia
Copy link
Member Author

mnencia commented Dec 4, 2024

Looks good. One comment is that we could have a begin time in ISO format, and an end time in non-ISO format, and the code would work fine. Would it make sense to add a consistency check? Either we use ISO format for both begin/end, or for neither. In any case, optional.

    if b.EndTimeISOString != "" {
         ... snipped ...
    } else if b.EndTimeString != "" {

If we add such a check, what is the desired behavior in case of inconsistency? I cannot imagine a situation where it happens, given that both the values are produced in a single berman-cloud-backup execution, so they are consistent by definition.

litaocdl
litaocdl previously approved these changes Dec 5, 2024
@mnencia mnencia dismissed stale reviews from litaocdl and jsilvela via e8abefd December 5, 2024 12:57
Signed-off-by: Marco Nenciarini <[email protected]>
Signed-off-by: Marco Nenciarini <[email protected]>
@mnencia mnencia merged commit d99f49b into main Dec 5, 2024
5 checks passed
@mnencia mnencia deleted the dev/48 branch December 5, 2024 13:57
@cnpg-bot cnpg-bot mentioned this pull request Dec 5, 2024
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.

Support ISO format for dates in the barman-cloud output

5 participants