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

feat: use craft-application project variables #4601

Conversation

cmatsuoka
Copy link
Contributor

Define version and grade as craft-application project variables that can
be updated using craftctl.

  • 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)

@cmatsuoka
Copy link
Contributor Author

Save the unit tests, save the world.

image

@cmatsuoka cmatsuoka requested review from mr-cal and lengau February 22, 2024 21:34
requirements-devel.txt Outdated Show resolved Hide resolved
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.09%. Comparing base (6ff3e0b) to head (623bb03).
Report is 16 commits behind head on feature/craft-application.

Additional details and impacted files
@@                      Coverage Diff                      @@
##           feature/craft-application    #4601      +/-   ##
=============================================================
+ Coverage                      88.83%   89.09%   +0.26%     
=============================================================
  Files                            327      332       +5     
  Lines                          22033    22081      +48     
=============================================================
+ Hits                           19573    19674     +101     
+ Misses                          2460     2407      -53     

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

@cmatsuoka cmatsuoka force-pushed the work/CRAFT-2469-support-adopt-info-and-dynamic-versions branch from c1f0300 to a454ec2 Compare February 22, 2024 23:47
@sergiusens sergiusens requested review from tigarmo and lengau and removed request for lengau February 23, 2024 15:16
@tigarmo
Copy link
Contributor

tigarmo commented Feb 23, 2024

As far as I can tell, the following tests are now OK regarding the adopt-info part (some are still failing for other reasons):

craftctl:test_snapcraftctl_compat (needs snapcraftctl)
craftctl:test_craftctl_get_set (I think theres a `snap install --edge core24` we can remove)
linters/library (still needs the gnome extension)
set-version-twice OK

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.

LGTM, please re-request a review from me when the craft-app PR lands

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka cmatsuoka force-pushed the work/CRAFT-2469-support-adopt-info-and-dynamic-versions branch from a454ec2 to da4d1bf Compare February 23, 2024 17:14
@cmatsuoka
Copy link
Contributor Author

Rebased on feature/craft-application.

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@tigarmo
Copy link
Contributor

tigarmo commented Feb 23, 2024

not sure how up-to-date/ready for review this is but the latest spread tests are all broken

2024-02-23T17:25:02.4667064Z 2024-02-23 17:25:02 Error restoring google:ubuntu-22.04-64:tests/spread/providers/lxd-build-info (feb231723-613888) : 
2024-02-23T17:25:02.4668796Z -----
2024-02-23T17:25:02.4669897Z + cd ../snaps/make-hello
2024-02-23T17:25:02.4670870Z + unset SNAPCRAFT_BUILD_ENVIRONMENT
2024-02-23T17:25:02.4671892Z + snapcraft clean --use-lxd
2024-02-23T17:25:02.4672740Z Traceback (most recent call last):
2024-02-23T17:25:02.4673684Z   File "/snap/snapcraft/x1/bin/snapcraft", line 5, in <module>
2024-02-23T17:25:02.4674813Z     from snapcraft.application import main
2024-02-23T17:25:02.4676427Z   File "/snap/snapcraft/x1/lib/python3.10/site-packages/snapcraft/application.py", line 108, in <module>
2024-02-23T17:25:02.4677780Z     APP_METADATA = AppMetadata(
2024-02-23T17:25:02.4679143Z TypeError: AppMetadata.__init__() got an unexpected keyword argument 'project_variables'
2024-02-23T17:25:02.4680280Z -----
2024-02-23 17:25:10 Successful tasks: 0

oh is this now depending on the craft-app merge?

@cmatsuoka
Copy link
Contributor Author

oh is this now depending on the craft-app merge?

The craft-app PR providing this functionality has already landed so I'm not sure what's happening here.

@tigarmo
Copy link
Contributor

tigarmo commented Feb 23, 2024

oh is this now depending on the craft-app merge?

The craft-app PR providing this functionality has already landed so I'm not sure what's happening here.

probably just have to update and re-rerun the tests

@cmatsuoka cmatsuoka requested a review from mr-cal February 24, 2024 20:02
@cmatsuoka
Copy link
Contributor Author

It seems to be good now.

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.

Spread test failures look as-expected. Thanks!

@cmatsuoka cmatsuoka marked this pull request as ready for review February 26, 2024 14:00
@sergiusens sergiusens merged commit b683207 into feature/craft-application Feb 26, 2024
8 of 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