Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

metainfo.xml: Add missing release timestamp attribute #4346

Merged
merged 1 commit into from
Oct 3, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Oct 2, 2021

Before, we only added the date attribute. The timestamp doesn't
carry any additional information, but it's required for flatpak:

https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/Flatpak.20build/near/255671054

@Holzhaus Holzhaus requested a review from Be-ing October 2, 2021 11:07
@Swiftb0y
Copy link
Member

Swiftb0y commented Oct 2, 2021

The python script produces different timestamps for the same date on CI compared to local?

@Holzhaus
Copy link
Member Author

Holzhaus commented Oct 2, 2021

The python script produces different timestamps for the same date on CI compared to local?

Oof, looks like I need to hardcode the timezone, otherwise it will interpret the date as local time.

@Holzhaus Holzhaus marked this pull request as draft October 2, 2021 12:44
@daschuer
Copy link
Member

daschuer commented Oct 2, 2021

https://freedesktop.org/software/appstream/docs/chap-Metadata.html#tag-releases

the timestamp tag contains the release time in the form of a UNIX epoch

The UNIX epoch is second since 1970-01-01 00:00 UTC

So there should be no issue to hard code it.

@Be-ing
Copy link
Contributor

Be-ing commented Oct 2, 2021

The timestamp isn't really needed despite the misleading error message I posted on Zulip. Flathub is fine with just the date attribute for a release.

@@ -98,7 +98,7 @@
Do not edit it manually.
-->
<releases>
<release version="2.3.1" date="2021-09-29" type="stable">
<release version="2.3.1" type="stable" date="2021-09-29" timestamp="1632866400">
Copy link
Member

Choose a reason for hiding this comment

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

1632866400 converts to 2021-09-28 22:00:00 not 2021-09-29
Can we just drop the redundant date property?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should do the opposite and drop the timestamp property.

Copy link
Member

Choose a reason for hiding this comment

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

If it is sufficient. I just thought timestamp it is mandatory for flathub.

attribute-missing : has no timestamp

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not just keep both? It's literally one line and since they are autogenerated, they can't get out of sync.

Before, we only added the `date` attribute. The `timestamp` doesn't
carry any additional information, but it's required for flatpak:

https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/Flatpak.20build/near/255671054
@Holzhaus Holzhaus force-pushed the metainfo-timestamp branch from 30652bd to d5ea821 Compare October 2, 2021 21:59
@Holzhaus Holzhaus marked this pull request as ready for review October 2, 2021 22:50
@Holzhaus
Copy link
Member Author

Holzhaus commented Oct 2, 2021

The python script produces different timestamps for the same date on CI compared to local?

Fixed.

@Holzhaus Holzhaus requested a review from daschuer October 3, 2021 11:26
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you.

@daschuer daschuer merged commit 9fe0128 into mixxxdj:2.3 Oct 3, 2021
@Holzhaus Holzhaus added this to the 2.3.2 milestone Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants