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

Fix the desktop file #1309

Merged
merged 1 commit into from
May 12, 2019
Merged

Fix the desktop file #1309

merged 1 commit into from
May 12, 2019

Conversation

bilelmoussaoui
Copy link
Contributor

If you run desktop-file-validate on the desktop file you will get this list of errors

 desktop-file-validate /usr/share/applications/webtorrent-desktop.desktop                                                         master    
/usr/share/applications/webtorrent-desktop.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
/usr/share/applications/webtorrent-desktop.desktop: error: file contains key "Path" in group "Desktop Action CreateNewTorrent", but keys extending the format should start with "X-"
/usr/share/applications/webtorrent-desktop.desktop: error: file contains key "Path" in group "Desktop Action OpenTorrentFile", but keys extending the format should start with "X-"
/usr/share/applications/webtorrent-desktop.desktop: error: file contains key "Path" in group "Desktop Action OpenTorrentAddress", but keys extending the format should start with "X-"

This PR does:

  • Remove the encoding key as it's not needed anymore
  • Remove the Path key as it's not required, the group actions inherit the properties of their "parent"

@stale
Copy link

stale bot commented May 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label May 10, 2018
@stale stale bot closed this May 17, 2018
@feross feross reopened this May 20, 2018
@stale stale bot removed the stale label May 20, 2018
@stale stale bot removed the stale label May 20, 2018
@bilelmoussaoui
Copy link
Contributor Author

Any news? I would like to work more on the linux standards in order to improve the usability of this application :)

@mathiasvr
Copy link
Contributor

@bilelmoussaoui If you don't mind that it can take a long time to get a PR merged, you are welcome to work on this! 👍
However, since there are fewer linux users, it can be more difficult to get feedback on your work.

@bilelmoussaoui
Copy link
Contributor Author

Yeah, totally got that. Except that the idea is to make the software easily available on Linux and working out of the box to bring more Linux users.
This should have been merged as I have mentioned the latest specs, the tool used to verify the desktop file, it can be tested within a docker container in a few minutes :)

@mathiasvr
Copy link
Contributor

The time it takes to test it is not the issue, webtorrent members have to actually do it, and this repository doesn't currently have much activity.
Also, I think many wouldn't simply accept a pull request based on running a tool on an OS they don't use regularly, so they also have to spend time looking into how this desktop file and it's attributes work.

Whether this should have been merged or not is not up to you, and that's why you should only work on this if you don't mind the long wait time.

@Borewit
Copy link
Member

Borewit commented Oct 18, 2018

This would help me #1502 to review on Fedora ;-)

@bilelmoussaoui
Copy link
Contributor Author

Just a quick update on this: here's the Desktop file specs: https://standards.freedesktop.org/desktop-entry-spec/latest/

Copy link
Member

@Borewit Borewit left a comment

Choose a reason for hiding this comment

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

I don't see a reason why not to include this

@Borewit Borewit merged commit 9a0a211 into webtorrent:master May 12, 2019
@welcome
Copy link

welcome bot commented May 12, 2019

🎉 Congrats on getting your first pull request landed!

@bilelmoussaoui bilelmoussaoui deleted the patch-2 branch May 12, 2019 18:32
@bilelmoussaoui
Copy link
Contributor Author

@Borewit Thanks for merging this!

@Borewit
Copy link
Member

Borewit commented May 13, 2019

Thanks for your effort @bilelmoussaoui.

@feross
Copy link
Member

feross commented Sep 7, 2019

@bilelmoussaoui Sorry for the long delay on this PR. If you'd like to continue improving Linux support, please do so! We'll try to be better about reviewing nice, small PRs like this one in a more timely manner. Thanks!

@bilelmoussaoui
Copy link
Contributor Author

bilelmoussaoui commented Sep 7, 2019

@feross Thanks for reaching. I just wanted to mention that WebTorrent is now packaged as a flatpak package (those new cool container based packaging formats) here https://flathub.org/apps/details/io.webtorrent.WebTorrent we are shipping a few files to make the app looks nice on every Linux software center downstream; would you mind having it here instead? https://github.com/flathub/io.webtorrent.WebTorrent/blob/master/io.webtorrent.WebTorrent.appdata.xml also I would give you write access if you want to automate the upgrade somehow :)

@feross
Copy link
Member

feross commented Sep 7, 2019

@bilelmoussaoui A few questions:

  • How do you build the Flatpack file?
  • Where is the flatpack file currently hosted? On Flathub? (We host all our release files in GitHub releases, so it would be best for us to host them there for consistency.)
  • How does Flathub learn that there's a new version available? (Ideally, it would periodically look at GitHub releases to check for new versions)

@bilelmoussaoui
Copy link
Contributor Author

The flatpak is built from a manifest, either a JSON or YAML file, https://github.com/flathub/io.webtorrent.WebTorrent/blob/master/io.webtorrent.WebTorrent.json

You can build it locally if you have flatpak-builder using
flatpak-builder --install app ./io.webtorrent.WebTorrent.json --user

Flatpak uses runtimes, which is basically a very simple container that contains the basics libraries to compile a new application. We have a BaseApp which is a bunch of shared modules used by a lot of apps but not enough to be a runtime. WebTorrent uses io.atom.electron.BaseApp

Currently, the package only extract the deb file & install it content. See the webtorrent module here https://github.com/flathub/io.webtorrent.WebTorrent/blob/master/io.webtorrent.WebTorrent.json#L20

I can make it build from the source code instead.

Regarding the updates, we can't "for now" look and auto-update packages just like that, as some other apps might not work with the latest runtime available, libraries... But you just need to update the URL pointing to the deb packages & sha256. You can either push directly to the master repository if you're sure that the build is working or open a PR & a bot will automatically build the package, provide you with a testing URL to try the app before merging & publishing your app see this MR for example flathub/com.spotify.Client#74.

Flathub is about stable, reproducible builds (but it has a beta branch support too). So the builds are built in Flathub structure build it for the 4 supported architectures & thanks to Flatpak the users of whatever distro can get the latest update no matter which distro they are using, if it supports Flatpak.

Don't hesitate to ping me for further questions.

@Borewit
Copy link
Member

Borewit commented Sep 7, 2019

This is what you are looking @feross: flathub/flathub#188

See also: Someone else has put my app on Flathub—what do I do?

@feross
Copy link
Member

feross commented Sep 7, 2019

@bilelmoussaoui Thanks for answering my questions. So if we move this manifest json file to this repo, like you're suggesting, will we be able to ship new versions to flathub users without sending a PR to the flathub repo?

@bilelmoussaoui
Copy link
Contributor Author

@feross You can't host the file here and have updates on Flathub without updating the file on Flathub repository. I can give you write access to whoever you want

@feross
Copy link
Member

feross commented Sep 10, 2019

@bilelmoussaoui I think it would be best if you guys and the community can handle keeping this file up to date since I don't have an easy way to test this.

@bilelmoussaoui
Copy link
Contributor Author

Sure, do you mind if I send a PR to add the appdata file that we have downstream? It's not a Flatpak specific thing, it's used to display the application info on almost any App Center on Linux. https://www.freedesktop.org/software/appstream/docs/chap-Quickstart.html#sect-Quickstart-DesktopApps

@feross
Copy link
Member

feross commented Sep 10, 2019

Sure! This should fix the problem where WebTorrent Desktop shows up as "Proprietary" software in the App Center, right? PR welcome!

@bilelmoussaoui
Copy link
Contributor Author

Yes! Perfect, I will prepare a PR for that whenever I can.

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

5 participants