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

flathub integration request - Nitrokey App #2081

Closed
wants to merge 6 commits into from
Closed

flathub integration request - Nitrokey App #2081

wants to merge 6 commits into from

Conversation

daringer
Copy link

Please confirm your submission meets all the criteria

  • I have read the App Requirements and App Maintenance pages.
  • My pull request follows the instructions at App Submission.
  • I am using only the minimal set of permissions.
    device=all was included in order to support the wide range of security-keys we are supporting now and in the future
  • All assets referenced in the manifest are redistributable by any party.
  • I am an upstream contributor to the project.
  • I own the domain used in the application ID or the domain has a policy for delegating subdomains (e.g. GitHub, SourceForge).
  • No additional patches or files have been submitted to the upstream projects concerned.

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37685

@flathubbot
Copy link

Build 37685 failed

@daringer
Copy link
Author

sorry my fault, did not run validate, now providing appdata through this repository, will be updated inside the application on next release, validate now also working:

$ flatpak run org.freedesktop.appstream-glib validate  build-dir/files/share/appdata/com.nitrokey.nitrokey-app.appdata.xml
build-dir/files/share/appdata/com.nitrokey.nitrokey-app.appdata.xml: OK

@barthalion
Copy link
Member

bot, build com.nitrokey.nitrokey-app

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37712

@flathubbot
Copy link

Build 37712 failed

README.md Outdated
@@ -0,0 +1,28 @@
# com.nitrokey.nitrokey-app - Nitrokey App Flatpak
Copy link
Member

Choose a reason for hiding this comment

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

This does not seem needed here.

Copy link
Author

Choose a reason for hiding this comment

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

done, removed

Makefile Outdated
@@ -0,0 +1,9 @@
local-install:
Copy link
Member

Choose a reason for hiding this comment

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

I'm usually against helper Makefiles in Flathub repos but it's up to you.

Copy link
Author

Choose a reason for hiding this comment

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

inherited from old non-flathub flatpak repo, personal anti-forget-mechanism, removed.

<screenshots>
<screenshot type="default">
<image>https://raw.githubusercontent.com/Nitrokey/nitrokey-app/master/images/icon/nitrokey-app-icon-128-black.png</image>
<caption><!-- Describe this screenshot in less than ~10 words --></caption>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<caption><!-- Describe this screenshot in less than ~10 words --></caption>

Copy link
Author

Choose a reason for hiding this comment

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

updated with 2 screenshots + captions

sources:
- type: file
path: com.nitrokey.nitrokey-app.appdata.xml
sha256: c7419a0cad827a1a5af82c8f0e6dfccfececa581fdb67a53bfefb0676b939f9d
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
sha256: c7419a0cad827a1a5af82c8f0e6dfccfececa581fdb67a53bfefb0676b939f9d

Local files don't need checksums, it's fine to drop it.

Copy link
Author

Choose a reason for hiding this comment

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

done

- --socket=x11
- --filesystem=host
- --device=all
modules:
Copy link
Member

Choose a reason for hiding this comment

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

You should add a global cleanup array to remove unneeded files, something like this is a good starting point:

cleanup:
  - "*.a"
  - /include

Copy link
Author

Choose a reason for hiding this comment

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

done and added more

- --device=all
modules:
- name: libusb
buildsystem: autotools
Copy link
Member

Choose a reason for hiding this comment

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

autotools is the default build system and doesn't need to be set explicitly.

Copy link
Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,3 @@
{
"only-arches": ["x86_64"]
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to limit it to x86_64?

Copy link
Author

Choose a reason for hiding this comment

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

as of now because it is not tested extensively

@daringer
Copy link
Author

thx for the feedback, updated and pushed

@bilelmoussaoui
Copy link
Member

bot, build com.nitrokey.nitrokey-app

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37742

@flathubbot
Copy link

Build 37742 failed

@daringer
Copy link
Author

Unfortunately there is no 64x64 icon included inside the latest release, pushed it into this repository. This will be fixed and then removed with our next release. Can I also start the build? bot, build com.nitrokey.nitrokey-app

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37747

@flathubbot
Copy link

Build 37747 failed

@daringer
Copy link
Author

struggling with icon paths, bot, build com.nitrokey.nitrokey-app

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37750

@flathubbot
Copy link

Build 37750 failed

@daringer
Copy link
Author

bot, build com.nitrokey.nitrokey-app

@flathubbot
Copy link

Queued test build for com.nitrokey.nitrokey-app.

@flathubbot
Copy link

Started test build 37753

@flathubbot
Copy link

Build 37753 successful
To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/36349/com.nitrokey.nitrokey-app.flatpakref

@daringer
Copy link
Author

daringer commented Jan 26, 2021

finally 😄, some more details:

  • the 64x64 icon is part of this repository, will be in the next release
  • same applies for the appdata xml file
  • testing using the build repository went fine

@nedrichards
Copy link
Member

/merge

@flathubbot
Copy link

A repository for this has been created: https://github.com/flathub/com.nitrokey.nitrokey-app

You will receive an invitation to be a collaborator which will grant you write access to the repository above. The invite can be also viewed here.

If you have never maintained an application before, common questions are answered in the app maintenance guide.

Thanks!

@flathubbot flathubbot closed this Jan 28, 2021
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.

5 participants