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

docs: migrate kde neon extensions #5220

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

medubelko
Copy link
Contributor

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

For review, please focus on docs/reference/extensions/kde-neon-extensions.rst. The other files will be merged through another PR.

This is a placeholder commit. Will be squashed when other extension work is merged.
Copy link

codecov bot commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.68%. Comparing base (654871d) to head (b2c0bf1).
Report is 681 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (654871d) and HEAD (b2c0bf1). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (654871d) HEAD (b2c0bf1)
2 1
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5220      +/-   ##
==========================================
- Coverage   94.88%   89.68%   -5.21%     
==========================================
  Files         658      342     -316     
  Lines       55189    22636   -32553     
==========================================
- Hits        52364    20300   -32064     
+ Misses       2825     2336     -489     

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

@medubelko
Copy link
Contributor Author

Will be reorienting around KDE 6 before opening for full review.

@medubelko medubelko marked this pull request as ready for review January 29, 2025 01:24
@medubelko medubelko changed the title Docs: Migrate KDE extensions docs: migrate kde neon extensions Jan 29, 2025
Runtime environment
~~~~~~~~~~~~~~~~~~~

The KDE neon extensions also sets various runtime environment variables for apps.

Choose a reason for hiding this comment

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

Suggested change
The KDE neon extensions also sets various runtime environment variables for apps.
The KDE neon extensions also set various runtime environment variables for apps.

Included layouts
----------------

The KDE neon 6 extension adds the following `layout

Choose a reason for hiding this comment

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

Suggested change
The KDE neon 6 extension adds the following `layout
The KDE neon 6 extension adds the following `layouts


Here's an example of the result of Snapcraft expanding project files that use the KDE
neon 5 and 6 extensions, immediately prior to build. It demonstrates the added plugs,
packages, variables, and layouts, that the extensions include in projects.

Choose a reason for hiding this comment

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

Suggested change
packages, variables, and layouts, that the extensions include in projects.
packages, variables, and layouts that the extensions include in projects.

@lengau lengau self-requested a review January 29, 2025 21:45
kcalc:
command: usr/bin/kcalc
extensions:
- kde-neon
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK whether this is relevant but we probably should use the neon 6 extension here if this file gets merged.

<https://snapcraft.io/kf5-5-113-qt-5-15-11-core22-sdk>`_

The extensions are designed for C++ based Qt/KDE Frameworks apps. They don't provide the
bindings needed in PySide2 (Qt for Python) or PyQt apps. The extensions also don't
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bindings needed in PySide2 (Qt for Python) or PyQt apps. The extensions also don't
bindings needed for PySide2 (Qt for Python) or PyQt apps. The extensions also don't

Comment on lines +148 to +151
For an example of a part's ``build-environment`` section modified by the KDE neon 6
extension, it sets the ``PATH``, ``XDG_DATA_DIRS``, and ``SNAPCRAFT_CMAKE_ARGS``
environment variables during build. These defaults can be overridden by being
pre-emptively declared in the project file.
Copy link
Contributor

Choose a reason for hiding this comment

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

These sentences sound a bit awkward

.. literalinclude:: ../code/extensions/kde-neon-extension-kcalc-expanded.diff
:language: diff
:lines: 3-
:emphasize-lines: 15-19, 27-28, 64-103
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:emphasize-lines: 15-19, 27-28, 64-103
:emphasize-lines: 15-19, 27-28, 58-103

.. group-tab:: KDE neon 6

The original is a `project file
<https://invent.kde.org/alexlowe/keysmith/-/blob/aml/snapcraft/snapcraft.yaml>`_
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

3 participants