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

Propose a "type" property to <release/> tag. #158

Merged
merged 3 commits into from Jan 24, 2018
Merged

Conversation

Jehan
Copy link
Contributor

@Jehan Jehan commented Dec 27, 2017

The logics behind this proposition is that in GIMP, we have development release. Someone is proposing us to add tag in our appdata. So far so good.

But he also wants items for the development releases. And that's where I am wondering if that is really relevant information. Well this is definitely relevant for anyone installing such development versions. But for most people, these are releases they will never see (and barely know they exist), and that's normal (distributions are certainly not expected to distribute them). Therefore if such a release listing (is there any software making usage of this tag by the way? GNOME Software 3.26 at least doesn't seem to do anything from it) were to include development versions, this would confuse people.

This is why I am proposing this change so that the tags can be properly categorized.

As a result, any software which is expected to make usage of the release listing should only show stable versions, unless development versions are distributed/installed.

Jehan added 2 commits December 27, 2017 20:03
To differentiate from dev or stable releases in particular.
@Jehan
Copy link
Contributor Author

Jehan commented Dec 27, 2017

Note: in my current proposition, I list 3 possible values for the type property: stable (the default when not setting the property), development and RC.

Actually I am thinking that maybe just "stable" and "development" could be enough (RC would just be categorized as a development release, which would be a broader naming for anything which is not "stable").
Just tell me if you prefer this approach and I can change this.

@hughsie
Copy link
Collaborator

hughsie commented Dec 28, 2017

I like stable and development, but not rc. It certainly makes it clear what the release notes are, and would be useful in gnome-software. @ximion makes the final call on the spec, but sounds good to me.

@Jehan
Copy link
Contributor Author

Jehan commented Dec 28, 2017

Done. No RC anymore. This was definitely too much.

I discovered of 2 more issues about the release tag, but I don't know if I should have them in separate pull requests or can just add commits here:

(1) I wanted to add URL to the release news but appstream-util strict validation would make an error if I add an URL in a <p> tag.
I propose we accept tags <url> inside <release> so that we can link a specific release to a news item on the website or release notes, or whatever makes sense.

(2) If we add a description to a release, we need to prepare it enough time before release so that translators can do their job on the <description> contents (in GIMP for instance, we are already "theoretically" in a string freeze so I'd want to have our description available to translators ASAP).
Unfortunately if we don't set a "date" property, once again, strict validation fails.
Additionally even if we set a future date, validation fails too.

And anyway, I don't think this is right to set a future date, nor a past date just to trick the system. This could be quite a problem if we release and forgot to set the <release> date right. I would definitely prefer we just forget the <release> tag altogether than having wrong dates.

So I propose to allow a <release> without a date property. Such a <release> tag would simply mean the associated version has not been released yet and installer should not show it in their listing.

If you are ok, I propose I also make these 2 changes in the repo.
I will happily propose a patch to appstream-util later as well if needed (when I can make the time).

@ximion
Copy link
Owner

ximion commented Dec 29, 2017

I was initially a bit skeptical about this, because a metainfo file is supposed to describe one software component - if it contains details of multiple branches of that software, it is harder to know which one we have now, because there will be two versions that are considered "current", one in the development branch and one in the stable branch - and we don't know which one we have if we have a metainfo file describing both.
But in the context of Flatpak & Co., I can see the merits of having such a distinction in the release notes. Metainfo files are also usually used in conjunction with a software delivery method, which provides the missing information of which version is current much better than AppStream could (and should).

So yeah, this looks good to me - thank you for writing the necessary documentation! I'll not merge this right away, since I am currently traveling much and want to think about this a bit more and ideally implement the feature when merging the spec change, as usual. So, I guess I'll get to this in the beginning of January.

@Jehan Point (1) requires a new thread/issue/PR, and for (2) I would actually like much more to see appstream-util be a little less strict instead of messing with the spec. For example, appstreamcli validate only checks that a valid date is present, but allows date in the future, specifically for the reasons you outlined - and because some CI systems turned out to have weird time settings. But that's ultimately @hughsie 's decision.

@Jehan
Copy link
Contributor Author

Jehan commented Dec 29, 2017

there will be two versions that are considered "current", one in the development branch and one in the stable branch - and we don't know which one we have if we have a metainfo file describing both.

This is a valid concern since I wondered myself, especially since I don't think that current appstream spec defines anywhere the version of the currently installed software, does it?

But then I also realized that anyway we obviously won't include 2.9.x dev <release> tags inside the appdata file for stable GIMP 2.8, so the 2.8 <release> tag will still be the newer one when installing this version. GIMP 2.9.x <release> listing will only be included in the appdata of a 2.9, 2.10 or later. In other words, the last version defined in the appdata is always the one currently installed (and that's also how you know if you installed a development or a stable version).

But of course, it would be even better if the appstream data had maybe a <version> tag or something, rather than having to resort to logics.

So yeah, this looks good to me […] So, I guess I'll get to this in the beginning of January.

Cool.

for (2) I would actually like much more to see appstream-util be a little less strict instead of messing with the spec. For example, appstreamcli validate only checks that a valid date is present, but allows date in the future, specifically for the reasons you outlined - and because some CI systems turned out to have weird time settings. But that's ultimately @hughsie 's decision.

About this, setting a date in the future is not always possible. Well it may be for perfectly oiled release process (typically in a company or a big project well organized). Typically in GIMP, we are never sure 100% when the release will be until the moment we actually make it (so we know the date for sure like in the last hours of making it). This is basically a process where most developers are volunteers, and so on. In such a context, I would say that setting no date is more suitable than setting a wrong date.

Also it makes sense (like semantically) that a release without a date is simply a release not done yet. I don't see anything bad, there, no? :-)
But yeah, it should be possible to set a date in the future for all these projects who are better organized than us, even with strict validation; yet I think also allowing no date should be valid as well.

@Jehan Jehan mentioned this pull request Dec 29, 2017
@ximion ximion merged commit fd576f2 into ximion:master Jan 24, 2018
ximion added a commit that referenced this pull request Jan 24, 2018
@hughsie
Copy link
Collaborator

hughsie commented Jan 25, 2018

I've just pushed support for this in appstream-glib too, thanks to both.

Jehan pushed a commit to GNOME/gimp that referenced this pull request Feb 1, 2018
- appstream-util returns a "style-invalid" error: "<ul> cannot start a
  description [(null)]". So I add a <p> introduction to the 2.9.8
  <release> tag. This was part of unit test failure on the appdata file.
- I also add a type property for 2.9.8. This is a new property which I
  proposed and which just got accepted in the appstream specification:
  ximion/appstream#158
- I add <release> tags for all previous 2.9.x releases. No description
  for these, just a type property. But feel free to propose patches
  adding short non-technical description for these.

Note: it was originally proposed in the bug report to use the appdata
file in place of NEWS (and have this one generated from appdata). But
after discussion with appstream project, appdata is expected to be
concise, non-technical and more "marketing" than exhaustive. This is
quite a different usage than NEWS which is more an exhaustive summary of
new features and major changes. So these 2 files will likely remain
distinct.
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.

None yet

3 participants