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

New kde-neon extension for Qt6 / KF6 #4698

Merged
merged 38 commits into from
Apr 10, 2024

Conversation

ScarlettGatelyMoore
Copy link
Contributor

  • [x ] Have you followed the guidelines for contributing?
  • [ x] Have you signed the CLA?
  • [x ] Have you successfully run tox run -m lint?
  • [ x] Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)

Tests pass locally.

Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.57%. Comparing base (6ff73a1) to head (a19d949).
Report is 4 commits behind head on main.

❗ Current head a19d949 differs from pull request most recent head c499e62. Consider uploading reports for the commit c499e62 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4698      +/-   ##
==========================================
+ Coverage   89.54%   89.57%   +0.03%     
==========================================
  Files         337      338       +1     
  Lines       22638    22711      +73     
==========================================
+ Hits        20271    20344      +73     
  Misses       2367     2367              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

@mr-cal mr-cal requested a review from lengau March 29, 2024 14:05
Copy link
Contributor

@lengau lengau left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

  • One related question (ABI stability)
  • One curiosity question
  • Two minor change requests to satisfy lawyers

extensions/desktop/kde-neon-6/locale-gen Show resolved Hide resolved
snapcraft/extensions/kde_neon_6.py Outdated Show resolved Hide resolved
snapcraft/extensions/kde_neon_6.py Show resolved Hide resolved
tests/unit/extensions/test_kde_neon_6.py Show resolved Hide resolved
ScarlettGatelyMoore and others added 9 commits April 2, 2024 04:24
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Co-authored-by: Alex Lowe <alex.lowe@canonical.com>
Remove the qt6 platform plug as we bundle the qt6 in the kf6 runtime due
to limited to one platform plug.
Copy link
Collaborator

@sergiusens sergiusens left a comment

Choose a reason for hiding this comment

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

great work!

Co-authored-by: Michał Sawicz <michal@sawicz.net>
@sergiusens
Copy link
Collaborator

seems like the connection is not happening in the test:

 + '[' 'ERROR: not connected to the kf6-core22 content interface.' = 'hello world' ']'

do we wait for this or manually connect for now? I am ok with either, but I am intending to cut a release tomorrow

@ScarlettGatelyMoore
Copy link
Contributor Author

seems like the connection is not happening in the test:

 + '[' 'ERROR: not connected to the kf6-core22 content interface.' = 'hello world' ']'

do we wait for this or manually connect for now? I am ok with either, but I am intending to cut a release tomorrow

I am good with manually connect if it's not to late.

@ScarlettGatelyMoore
Copy link
Contributor Author

I just had to add fix 2b439f9e75c1e05e1aa5219a455c8239c3e79f9a to our $HOME ref: https://forum.snapcraft.io/t/setting-home-in-snap-to-user-home-seems-to-cause-all-empty-user-dirs-to-become-broken-symlinks/39463/5

@sergiusens
Copy link
Collaborator

@ScarlettGatelyMoore manual connect is good if you can do it now

@sergiusens sergiusens merged commit 3d12a68 into canonical:main Apr 10, 2024
10 checks passed
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