elementary / appcenter-reviews Goto Github PK

What Happened?

The only examples I see of EOL apps still show their pre-EOL updates in AppCenter, and don't warn when installing or updating from the CLI.

• Dippi
• Clairvoyant
• Principles
• Palette
• LookBook

In all cases, the apps are showing the release information before their respective EOL update in the remote's AppStream data. For example, installing or updating LookBook doesn't recommend installing Icon Browser from the Flatpak CLI, and Clairvoyant is still showing its 2.0.0 release, meaning the 2.0.1 release never made it into the remote.

Steps to Reproduce

1. Mark an app as EOL in this repo, either with no other updates (like Dippi), with a version bump (like Clairvoyant), or with a rebase (like LookBook).
2. Merge it
3. See that the app's update never comes through to the AppCenter remote, AppStream data, Flatpak CLI, AppCenter native client, or AppCenter website

Expected Behavior

Marking an app as EOL in this repo should propagate that information to the Flatpak remote.

OS Version

7.x (Horus)

Software Version

Latest release (I have run all updates)

Log Output

No response

Hardware Info

No response

Problem

It's not always immediately clear if a PR is a new app, and update, requires manual RDNN verification, etc.

Proposal

We can use labels to organize PRs and as a visual reminder for some of those things, like:

• RDNN verification
  • Requires RDNN Verification
  • RDNN Verified (e.g. submitted by dashboard)
• Type of submission
  • New App
  • Update

There may be more things that would be useful to flag, but these are the ones I can think of off the top of my head.

Prior Art

What Happened

An app dev appears to have rewritten a release tag; #66. The parse and build actions happily succeeded even though the listed commit doesn't match the release's commit.

Expected Behavior

The parse action should check the commit of the release and fail if it doesn't match.

We keep getting devs confused about app submission and trying to use the old dashboard.

According to https://docs.flatpak.org/en/latest/conventions.html, you should use "io.github.[username].[app]" not "com.github.[username].[app]"

What Happened?

AppCenter makes frequent use of 48px icons in lists, however only 64px and 128px icons are served from the remote even if the app has more icon sizes available.

Steps to Reproduce

See https://flatpak.elementary.io/repo/appstream/x86_64/icons/

Expected Behavior

We should provide 48px icons from the remote as well so that we don't get blurry icons in lists or have to bump up to significantly larger 64px icons

OS Version

7.x (Horus)

Software Version

Latest release (I have run all updates)

Log Output

No response

Hardware Info

No response

As seen in bcedu/ValaSimpleHTTPServer#8

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Problem

In elementary there aren't some important utilities: e.g. Document Scanner, Disk utility and Maps.
The related Flathub versions do not follow the system GTK4 theme.

Proposal

I think the first one is the most important and essential, but I invite you to consider the others as well. You could do the same work that has already been done with File Roller, Fonts and Evince.

Prior Art (Optional)

No response

Updates are not available for the following applications, although the corresponding pull requests were successfully merged a few days ago:
com.github.alexkdeveloper.guessnumber
com.github.alexkdeveloper.bmi
com.github.alexkdeveloper.pasgen

Yesterday (or 18 hours ago) I created release tag 1.145.0 and submitted it to the dashboard at https://beta.developer.elementary.io/submissions. I got a message that my app has been submitted for review and will appear as a pull request in the review queue shortly.

On the dashboard itself a section called Recent Apps appeared listing a generic app icon without any info and only a label saying Processing Releasing:

After a few hours the Recent Apps listing was gone and no pull request was ever created... no feedback, nothing.

Problem

We have to check a bunch of places for info like the app's name, categories, etc.

Proposal

It might be handy to automatically aggregate that info into one place and drop it as a comment on the PR.

Prior Art (Optional)

No response

We should warn developers if their appdata summary or description contains "for elementary" or "for Linux". Maybe something like:

Don't call out the platform in your AppData

Since all apps in AppCenter are built for elementary OS, calling out the platform in your app's description or summary doesn't describe something unique about your app. Focus on describing what your app empowers users to do or helps them avoid doing.

---

For more tips about creating a great AppCenter listing, check out this post on Medium

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

According to https://github.com/flatpak/flatpak/wiki/TestingPurchases, we need to specify that the things we build into the repo require a token

add this line to the example repo config here:

 "require-auth-for-token-types": [2],
 This will make flat-manager require a token matching the ref for all commits with token-type 2.

Now, build something we want to test, I'm using eye of gnome as an example here:

$ flatpak-builder --repo build-repo builddir org.gnome.eog.yml

Then import and publish the build into flat-manager with a token type of 2:

$ export REPO_TOKEN=$(echo -n "secret" | base64 | cargo run --bin gentoken -- --base64 --secret-file - --name test)
$ flat-manager-client push --publish --wait-update --token-type=2 $(flat-manager-client create http://127.0.0.1:8080 stable) build-repo

We can verify that this commit gets a 403 error: (replace with your commit ids)

$ cat repo/refs/heads/app/org.gnome.eog/x86_64/master 32820032f58cb1229edffdc66dc829b29a8cce9f8b6d9f36342d76a004b61d01
$ curl http://127.0.0.1:8080/repo/stable/objects/32/820032f58cb1229edffdc66dc829b29a8cce9f8b6d9f36342d76a004b61d01.commit
{"error-type":"token-insufficient","message":"Not enough permissions: No token specified","status":403}

What Happened

Flatpak, due to it following freedesktop and dconf spec, cannot have hyphens in the username part of the reverse-DNS, so flatpak recommends using underscores. However, GitHub does allow hyphens in names, but doesn't allow underscores - so you can see the issue.
Here is a perfect example of this problem already with an AppCenter developer

Expected Behavior

Dashboard warns that hyphened names should use underscores instead in apps

The AppCenter client only supports prices in integer dollars (by design). However, some apps have non-integer prices: https://github.com/kjlaw89/archetype

AppCenter currently just takes the int and lops off the decimal, which is probably not what a developer would expect. We should validate this in the AppData at build time so the devs aren't setting unexpected prices.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Problem

For the API, we need to track the first release somewhere—it makes sense to keep that data here with all the other app data.

Proposal

Add a first-released-at key to the JSON files that contains an ISO8601 datetime of the first time that RDNN was merged into master.

Prior Art

We previously used a database to store the first release dates.

Right now all the commit subjects are in the format Export com.github.cassidyjames.dippi which makes it difficult to identify which commit points to which version of the app, which is needed to install a specific version.

To reproduce

1. Run flatpak remote-info --log appcenter com.github.cassidyjames.dippi or the same command for any other app
2. Look at the subjects

Proposal

Add the app's version number to the commit subject.

Problem

Until recently, only apps with an RDNN were accepted according to the following scheme: com.github.<user>.<repository>.

However, it should be possible to transfer an already published app to its own domain.

Proposal

It should be possible to specify the new RDNN with the entry end-of-life-rebase when a reason for end-of-life is specified.

Prior Art (Optional)

Flatpak

hm, so instead of removing the old file, we need to rebuild that package with end-of-life-rebase as an argument to flatpak builder: https://docs.flatpak.org/en/latest/flatpak-command-reference.html?highlight=end-of-life-rebase

I think you need to file a new feature request against this repo where you can add end-of-life and end-of-life properties to the json file. And I'm also not sure if the Flatpak builder action supports these parameters, so we might need an upstream feature request there as well

Posted by @danrabbit in #292 (comment)

Flathub

There may come a point where an application is no longer maintained. In order to inform users at update or install time that it will no longer get updates, create flathub.json with these contents:

{
  "end-of-life": "This application is no longer maintained because..."
}

If the application has been renamed, you can additionally include end-of-life-rebase with the new ID. Recent flatpak versions will prompt user if they'd like to switch to the renamed app.

{
  "end-of-life": "The application has been renamed to...",
  "end-of-life-rebase": "the.new.appid"
}

Source: https://github.com/flathub/flathub/wiki/App-Maintenance#end-of-life

It doesn't look like we're generating screenshots for pushed apps?

https://flatpak.elementary.io/repo/screenshots/

• Do not start the description with the application name, we already have the name written above in all contexts.
• Do not mention the language it is written into: we probably want to have a warning when the keywords "Vala", "Python", "Rust", "Javascript"… are used

Problem

We have to manually check that the app uses the elementary Platform as its runtime (see: #65). It adds another manual step.

Proposal

We could use the build action to automatically check, maybe?

Prior Art

Problem

Currently, there is confusion about whether or not unsandboxed apps are allowed on the elementary OS AppCenter.

This stems from a historic misunderstanding of what constitutes an unsandboxed Flatpak app which is basically any app that has --filesystem=home or above permissions. This is the level of permissions necessary for a malicious app to escape its sandbox and thus the level of permissions at which an app should be considered unsandboxed.

This leads to confusion where a new unsandboxed app can be refused a listing on AppCenter (e.g., see #225) whereas a cursory review reveals that are already unsandboxed apps on the AppCenter: https://gist.github.com/aral/5b283d020bab878ca8b2cc83bc66b88d

Proposal

• Document that --filesystem=home and above permissions in a Flatpak app means that the app is unsandboxed.
• Write a policy regarding whether unsandboxed apps are accepted on AppCenter.
• Label apps as sandboxed or unsandboxed accordingly in AppCenter.

There are valid use cases for having unsandboxed apps on AppCenter (one example is a developer tool like Comet – https://comet.small-web.org, AppCenter review details in pull request linked above) and it is my personal opinion that they should be included.

If, however, it is decided that unsandboxed apps are not included in AppCenter, there needs to be a review of existing apps so that unsandboxed ones are removed.

Gotchas to avoid

In terms of whether an app is sandboxed or not, it is important to understand that what matters is whether it has --filesystem=home permissions. Anything above this (e.g., --filesystem=host) is no more or less sandboxed.

Other considerations

While defining whether an app is sandboxed or not, we also have to take into consideration that elementary OS currently uses X11, not Wayland. So any app that uses X11 isn’t really sandboxed and we should decide how this is communicated so as not to create a false sense of security (see prior art, below).

Prior Art (Optional)

See #360

It looks like Memories has a release with its release date in the future :p It's listed as April 25th 2018

https://appstream.elementary.io/appcenter/html/xenial/main/metainfo/com.github.artemanufrij.showmypictures.html

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Right now AppData URLs (i.e. issue tracker, homepage, translations) can be inserted without protocols; see heisantosh/qrshare#22. This means they aren't actually usable inside of AppCenter. We should enforce the inclusion of protocols with a test in Houston.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

I thought this existed, but I've come across a couple of updates in the reviewer dashboard that lacked AppStream release information for the release being published. We should fail and file an issue that release notes are always required for the latest release

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

We currently require this since the applications launcher is where users go to see their apps. We might remove this requirement at a later date if we figure out a better story around these types of apps, but we've been enforcing it for now.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Problem

We have to step through the same review steps on every PR which is hard to always remember. Plus, it's not as transparent to the public where a review is unless we comment as we review it.

Proposal

Have an action to automatically throw a checklist into the PR body so we can check those off as it's reviewed.

Prior Art

I've seen actions that comment on a PR with a checklist, but using the PR body is nice because GitHub considers it the overall progress for the PR in the header and on the PRs page.

These colors should pass WCAG AA at least

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Not sure if it should be from our account or a new account, but it would be rad if we automatically tweeted out the AppCenter link whenever a new app was published.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Someone tried to submit an app with our StackExchange as their help link. We should probably check and make sure that developers aren't using any elementary-owned URLs as their help link like:

• elementary.io
• elementaryos.stackexchange.com
• reddit.com/r/elementaryos/
• https://plus.google.com/communities/104613975513761463450

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

The Setup action (which adds the checklist to the PR description) fails when the PR is coming from a fork, e.g. when a developer submits it themselves. See: #100 and the initial CI run on #98