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

Add org.photoqt.PhotoQt #292

Closed
wants to merge 5 commits into from
Closed

Add org.photoqt.PhotoQt #292

wants to merge 5 commits into from

Conversation

luspi
Copy link

@luspi luspi commented Mar 5, 2018

PhotoQt is a different kind of image viewer. Website: https://photoqt.org

@@ -0,0 +1,40 @@
{
"id": "org.qt.photoqt",
Copy link
Member

Choose a reason for hiding this comment

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

org.photoqt.PhotoQt

"rename-desktop-file": "photoqt.desktop",
"rename-appdata-file": "photoqt.appdata.xml",
"rename-icon": "photoqt",
"finish-args": ["--share=ipc","--socket=x11","--socket=wayland","--filesystem=host","--device=dri","--socket=pulseaudio"],
Copy link
Member

Choose a reason for hiding this comment

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

Please put these on their own line. Same applies for everything else and broken whitespace throughout.

"modules": [
{
"name": "exiv2",
"cmake": true,
Copy link
Member

@TingPing TingPing Mar 5, 2018

Choose a reason for hiding this comment

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

Use "buildsystem": "cmake-ninja",. Same for the other.

},
{
"name": "libraw",
"cmake": false,
Copy link
Member

@TingPing TingPing Mar 5, 2018

Choose a reason for hiding this comment

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

That is the default, can be removed. Same for the other.

@TingPing
Copy link
Member

TingPing commented Mar 5, 2018

bot, build org.photoqt.PhotoQt

https://flathub.org/builds/#/builders/1/builds/2430

@TingPing TingPing changed the title Add PhotoQt to Flathub Add org.photoqt.PhotoQt Mar 5, 2018
"--share=ipc",
"--socket=x11",
"--socket=wayland",
"--filesystem=host",
Copy link
Member

Choose a reason for hiding this comment

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

this seems like too broad a privilege request

Copy link

Choose a reason for hiding this comment

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

Hi, I'm the one who updated manifest to be Flathub ready. I'm not sure PhotoQt can work in full sandbox via portal, so there needs to be direct access to the filesystem. Access only to home would be fine? The original manifest had "host" and I thought the app author wanted it to browse outside home as well, so I kept it there.

Copy link
Member

Choose a reason for hiding this comment

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

@luspi what's the expected use case here? xdg-pictures and videos? home? viewing random assets from the rest of the system?

Copy link
Author

Choose a reason for hiding this comment

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

I think "viewing random assets" is a pretty good description of its use has. It includes of course folders like xdg-pictures and home, but also goes beyond that to anywhere a use might encounter an image of some type...

@nedrichards
Copy link
Member

The JSON file needs to be the same as the app-id and org.qt.photoqt.json isn't org.photoqt.PhotoQt so the build fails https://flathub.org/builds/#/builders/76/builds/21

@nedrichards
Copy link
Member

bot, build org.photoqt.PhotoQt

@nedrichards
Copy link
Member

nedrichards commented Mar 6, 2018

Looks like there's something wrong in the section of the appdata:

Error loading AppData file: failed to parse /app/share/appdata/org.photoqt.PhotoQt.appdata.xml: Error on line 81 char 6: already set '​' and tried to replace with '

https://flathub.org/builds/#/builders/4/builds/2195 e.g.

Other than that, everything looks like it built perfectly.

@Sesivany
Copy link

Sesivany commented Mar 6, 2018

I copied that section from a different file and it has probably introduced some non-visible characters that shouldn't be there. When I removed all the spaces, the file gets validated. I also noted that if doesn't have any nested elements, which it doesn't in this case because I didn't include download/installed sizes, it can self-close. So that part can be simplified into this:

<releases>
<release version="1.6" date="2018-02-20" />
</releases>

@luspi
Copy link
Author

luspi commented Mar 6, 2018

Simplified release section, and found the "faulty" spaces in the appdata file. It should (hopefully) work now!

@nedrichards
Copy link
Member

bot, build org.photoqt.PhotoQt

@nedrichards
Copy link
Member

Repository has been created: https://github.com/flathub/org.photoqt.PhotoQt

@nedrichards nedrichards closed this Mar 8, 2018
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

4 participants