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.

Sign up for GitHub

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

Jump to bottom

Sgmoore/update neon ext core22 #4023

Merged
merged 59 commits into from
Feb 23, 2023
Merged

Sgmoore/update neon ext core22 #4023

merged 59 commits into from
Feb 23, 2023

Conversation

ScarlettGatelyMoore
Copy link
Contributor

  • [x ] Have you followed the guidelines for contributing?
  • [ x] Have you signed the CLA?
  • Have you successfully run make lint?
  • Have you successfully run pytest tests/unit?

Needs global set for tests to pass. Store request is in.

@lengau
Copy link
Contributor

lengau commented Jan 25, 2023

Please fix the linting issues. If you've got tox 4 on your machine you can check it locally with tox run -m lint

ScarlettGatelyMoore and others added 20 commits January 25, 2023 10:47
@ScarlettGatelyMoore
Fix lint tests.
96a1396
@ScarlettGatelyMoore
Merge branch 'main' into sgmoore/update_neon_ext_core22
f4681a8
@ScarlettGatelyMoore
Fix black test.
437bbbe
@ScarlettGatelyMoore
Fix syntax found by black test.
2739903
@ScarlettGatelyMoore
Fix syntax found by black.
014b184
@ScarlettGatelyMoore
Fix kde-neon ext test formatting.
aabd50f
@ScarlettGatelyMoore
Also rename the test with underscore kde_neon
0764381
@ScarlettGatelyMoore
Fix expectation yaml for kde-neon-extension and make-parameters in kd…
bdbef5c 
…e_neon test
@ScarlettGatelyMoore
Formatting! kde-neon test.
918bde1
@ScarlettGatelyMoore
More formatting fixes for kde_neon ext and test.
db1e9c3
@ScarlettGatelyMoore
kde-neon extension test formatting
ea4fa1b
@ScarlettGatelyMoore
Remove f-strings, fix build-environment test.
e86c36e
@ScarlettGatelyMoore
Fix formatting kde-neon test.
9fa24f1
@ScarlettGatelyMoore
Remove f-strings kde-neon ext, sort import block found by ruff.
3a64f3a
@ScarlettGatelyMoore
Merge branch 'snapcore:main' into sgmoore/update_neon_ext_core22
794db89
@ScarlettGatelyMoore
Fix from tox -m fix kde-neon-extention test.
f17f73e
@ScarlettGatelyMoore
Fix pylint line-to-long errors.
5526eac
@ScarlettGatelyMoore
Fix formatting with black test, kde-neon test.
17156af
@ScarlettGatelyMoore
Fix docstring test for kde-neon extension.
29dfe65
@ScarlettGatelyMoore
Remove the extra blank line after docstring, kde-neon ext.
1242aaf
@ScarlettGatelyMoore
Copy link
Contributor Author

I am pretty certain the rest of the tests require the global set on the content pack.

ScarlettGatelyMoore and others added 21 commits February 4, 2023 07:09
@ScarlettGatelyMoore
kde-neon-extension: only run spread test on core20 until global is se…
5667f00 
…t on core22.
@ScarlettGatelyMoore
kde-neon-extension: spread test, remove the -all from connect.
e7ca659
@ScarlettGatelyMoore
kde-neon-extension: fix list_registry command test.. I hope.
2abae68
@ScarlettGatelyMoore
kde-neon-extension: ebuild-environment assert again, either I fail li…
633add3 
…nter for line to long or I fail this test.
@ScarlettGatelyMoore
Merge branch 'main' into sgmoore/update_neon_ext_core22
4b82e43
@ScarlettGatelyMoore
kde-neon-extension: fix various tox tests.
47000a1
@ScarlettGatelyMoore
kde-neon-extension: fix black linter.
7ff11d0
@ScarlettGatelyMoore
kde-neon-extension: fix pylint line to long.
d7f46f7
@ScarlettGatelyMoore
kde-neon-extension: fix spread test, remove the connect as we use pla…
d6ebe33 
…tform plug. Fix the build snaps name.
@ScarlettGatelyMoore
kde-neon-extension: remove cruft.. :(
0d84452
@ScarlettGatelyMoore
kde-neon-extension: fix spacce for black linter.
221a717
@ScarlettGatelyMoore
kde-neon-extension: fix sdk_different_channel test.
1b39022
@ScarlettGatelyMoore
kde-neon-extension: and... changing make-parameters back again to mak…
1d7c167 
…e black happy. I wish it would make up its mind which way to do it.
@ScarlettGatelyMoore
kde-neon-extension: fix spaces and line breaks in build-environment.
598d76b
@sergiusens
Merge remote-tracking branch 'origin/main' into sgmoore/update_neon_e…
146e628 
…xt_core22
@sergiusens
lint: black
edc9478
@ScarlettGatelyMoore
kde-neon-extension: fix tests with proper test environment.
4cd1e0f
@ScarlettGatelyMoore
kde-neon-extension: fix indentation.
9f04fe5
@ScarlettGatelyMoore
kde-neon-extension: fix pythonpath.
8bda529
@ScarlettGatelyMoore
kde-neon-extension:use join for pythonpath.
366f15e
@sergiusens
update tests, remove uneeded exports
dc7160f 
- remove PYTHONPATH, it is not in the SDK
- GETTEXT is also not in the SDK
- Simplify string operations

Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
@sergiusens
Copy link
Collaborator

I have done the last bit of test updates, will keep an eye on spread integration tests. I would encourage the rename of the snaps to

  • kf5-qt%QT_VERSION%-sdk
  • kf5-qt%QT_VERSION%

If the kf5 version is not as important; this would save the need to rebuild all snaps all the time, save us from requesting default connections and take advantage of delta downloads for the content snap

@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2023 •
edited
Loading

Codecov Report

Merging #4023 (dc7160f) into main (46791aa) will increase coverage by 0.07%.
The diff coverage is 95.77%.

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

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #4023      +/-   ##
==========================================
+ Coverage   91.68%   91.75%   +0.07%     
==========================================
  Files          78       79       +1     
  Lines        5230     5301      +71     
==========================================
+ Hits         4795     4864      +69     
- Misses        435      437       +2
Impacted Files Coverage Δ
snapcraft/extensions/kde_neon.py 95.71% <95.71%> (ø)
snapcraft/extensions/registry.py 90.47% <100.00%> (+0.47%) ⬆️
snapcraft/commands/extensions.py 100.00% <0.00%> (+1.88%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ScarlettGatelyMoore
Copy link
Contributor Author

I have done the last bit of test updates, will keep an eye on spread integration tests. I would encourage the rename of the snaps to

* `kf5-qt%QT_VERSION%-sdk`

* `kf5-qt%QT_VERSION%`

If the kf5 version is not as important; this would save the need to rebuild all snaps all the time, save us from requesting default connections and take advantage of delta downloads for the content snap

Hi Sergio, as Harald mentioned in the channel, users would end up with weird bugs if the content snap kf5 versions don't exactly match the versions the applications are built with. So we have to keep this format. However, I can get away with shortening kde-framworks... to kde-kf5 . It won't help with the store wait time or provisional updates sadly.

@sergiusens
Copy link
Collaborator

@ScarlettGatelyMoore let's make sure the next update has the proper name then. Also, is the content snap clear for global connection?

@sergiusens sergiusens enabled auto-merge (squash) February 23, 2023 19:14
@sergiusens sergiusens merged commit 4e50340 into canonical:main Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lengau lengau lengau left review comments

@sergiusens sergiusens sergiusens approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ScarlettGatelyMoore @lengau @sergiusens @codecov-commenter