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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃毀馃彈馃懛 Accept and deliver What's New information #580

Closed
wants to merge 3 commits into from

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Sep 24, 2018

馃毀 WIP solves #578 馃毀

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@BernhardPosselt
Copy link
Member

BernhardPosselt commented Sep 25, 2018

What about simply parsing https://keepachangelog.com/en/1.0.0/ which is already supported by the store?

@blizzz
Copy link
Member Author

blizzz commented Sep 26, 2018

What about simply parsing https://keepachangelog.com/en/1.0.0/ which is already supported by the store?

That's too detailed. The idea is to provide up to 3 easy-understandable, non-technical items for end users (plus three for admins). Sort of release highlights.

@BernhardPosselt
Copy link
Member

Have you thought about using dashes instead of camel case? e.g. <whats-new> instead of <whatsNew> etc?

<xs:element name="regular" type="whatsNewInfo" minOccurs="1" maxOccurs="1" />
<xs:element name="admin" type="whatsNewInfo" minOccurs="0" maxOccurs="1" />
</xs:sequence>
<xs:attribute name="lang" type="language" use="required" />
Copy link
Member

Choose a reason for hiding this comment

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

@MorrisJobke how would this interact with translation? Not sure what your plans are currently for translating xml contents.

Copy link
Member

Choose a reason for hiding this comment

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

<xs:simpleType name="version">
<xs:restriction base="xs:string">
<xs:pattern value="(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)"/>
<xs:minLength value="6"/>
Copy link
Member

Choose a reason for hiding this comment

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

I'd drop the min and max length requirements. min length of 6 makes 0.0.1 invalid btw.


<!-- based on xs:language, just uses _ as a separator between lang and region. -->
<xs:simpleType name="language">
<xs:restriction base="xs:token">
Copy link
Member

Choose a reason for hiding this comment

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

<xs:element name="whatsNew" minOccurs="1" maxOccurs="unbounded">
<xs:complexType>
<xs:sequence>
<xs:element name="regular" type="whatsNewInfo" minOccurs="1" maxOccurs="1" />
Copy link
Member

Choose a reason for hiding this comment

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

What if there are only admin changes? like "ui for configuring additional mount type"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we should consider that. Might make sense for the server, too, as the maintenance releases often don't have much to brag about, but subtle bug fixes.

@@ -35,6 +35,9 @@ def get_release_info(self, url: str, is_nightly: bool = False) -> Release:
info = parse_app_metadata(meta.info_xml, self.config.info_schema,
self.config.pre_info_xslt,
self.config.info_xslt)
if meta.changes_xml:
whats_new = parse_app_whats_new(meta.changes_xml, self.config.changes_schema,
Copy link
Member

Choose a reason for hiding this comment

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

Looks like an unused variable. If it's just for validation simply remove the assignment

Copy link
Member Author

Choose a reason for hiding this comment

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

still WIP

<item>First {{ app.name }} release!</item>
</regular>
<admin>
<item>requires {{ app.nextcloud_version }}</item>
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant with the version in info.xml and probably a bad example for new developers. Better example or simply remove it?

<xs:element name="release">
<xs:complexType>
<xs:sequence>
<xs:element name="changelog" maxOccurs="1" minOccurs="1">
Copy link
Member

Choose a reason for hiding this comment

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

This could be optional and re-use the CHANGELOG.md in the app folder by default
.

@blizzz
Copy link
Member Author

blizzz commented Oct 8, 2018

fyi, i was sick and need to reschedule a bit. so, will be a bit until i pick it up again.

@alexbijma
Copy link

hi, is there any progression in this? This would be very very useful to have.

@blizzz blizzz closed this Mar 12, 2024
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.

None yet

4 participants