flathub-infra / flatpak-external-data-checker

A tool for checking if the external data used in Flatpak manifests is still up to date
GNU General Public License v2.0
115 stars 36 forks source link

creates empty description tags #422

Open Mailaender opened 5 months ago

Mailaender commented 5 months ago
    <release version="0.16.5" date="2024-05-26">
      <description></description>
    </release>
bbhtt commented 5 months ago

This is expected and was introduced recently to encourage people to fill a release description in rather than leaving it empty.

Mailaender commented 5 months ago

Okay, then that is terrible design. Consider something like

    <release version="0.16.5" date="2024-05-25">
      <url>https://sameboy.github.io/changelog/</url>
    </release>

with a static URL or auto-generated from GitHub releases

    <release version="0.16.5" date="2024-05-25">
      <url>https://github.com/LIJI32/SameBoy/releases/tag/v0.16.5</url>
    </release>

instead.

bbhtt commented 5 months ago

Most tags on GitHub have no release notes, that is not very useful. And it doesn't work for other sources like archive.

Is an empty description creating problems? You can just ignore it.

Mailaender commented 5 months ago

I now have to manually remove it to avoid empty tags. Definitely won't write changelogs when the maintainer published none. I also know of no one who writes changelogs exclusively for @flathub.

Mailaender commented 5 months ago

You are encouraged enough by the "No changelog provided" on the https://flathub.org/apps site.

bbhtt commented 5 months ago

Ah so it is the tag-empty error from appstream right?

cc @wjt I'll revert the flatpak to an old commit which will fix the issue.

bbhtt commented 5 months ago

I now have to manually remove it to avoid empty tags

Ah so it is the tag-empty error from appstream right?

Looks like it passes the linter validation. The empty tag does not create any issues. So reverting is not needed.

So you can just ignore it?

cc @wjt

Sorry for the ping! I panicked a bit 😅

Mailaender commented 5 months ago

It is maybe just a syntax polish problem, but it annoys me so much that I will disable x-data-checker now.

wjt commented 5 months ago

@razzeee Early feedback on this change suggests that this is having the opposite effect to what was intended. WDYT?

razzeee commented 5 months ago

I now have to manually remove it to avoid empty tags. Definitely won't write changelogs when the maintainer published none. I also know of no one who writes changelogs exclusively for @flathub.

that's not what anyone was suggesting. the problem is, that no fedc provider can actually get us a changelog to automatically put here.

so the next best thing is to automatically add this tag, so it's waaay easier to fill (as in copy from upstream) without having to remember, the format of the needed xml items. Having to touch each x-data-checker entry was annoying before, but this makes it a bit less annoying. And it leads to less errors due to wrong formating.

@wjt I'm not sure I would say that, main intend was making adding changelogs easier. Encouraging people is slightly different.

Mailaender commented 5 months ago

You can make this feature optional with a flag in flathub.json:

{
  "prefill-empty-changelog": true
}

especially since it conflicts with

{
  "automerge-flathubbot-prs": true
}

I also discovered it in https://github.com/flathub/com.microsoft.EdgeDev/pull/81 where I then had to remove the junk tags.

wjt commented 5 months ago

Is the empty <description> tag actually rejected by the appstream validator?

bbhtt commented 5 months ago

No it doesn't, I just panicked thinking something else. It doesn't make sense to have an option more such a minor "stylistic" thing, imo.

razzeee commented 5 months ago

"automerge-flathubbot-prs": true

Will be removed soonish from my understanding