work/CRAFT-3862-Implement-the-initial-Imagecraft-spec #70

upils · 2025-01-08T13:04:28Z

Have you followed the guidelines for contributing?
Have you signed the CLA?

Specification https://docs.google.com/document/d/1pTXN9LIFu4F0KmCT-iK2-_gGKELapKxvEj0-74xdr3g/edit?tab=t.0

.pre-commit-config.yaml

common.mk

mattculler

A link to the spec would be helpful
Love to see thousands of lines deleted!
It looks like this PR includes both a merge from starbase and spec implementation - in the future, splitting those two large changes into separate PRs would make for easier review.

But looks good! None of my inline comments are blockers.

HACKING.rst

common.mk

imagecraft/models/project.py

pyproject.toml

upils · 2025-01-16T08:54:59Z

A link to the spec would be helpful

Love to see thousands of lines deleted!

It looks like this PR includes both a merge from starbase and spec implementation - in the future, splitting those two large changes into separate PRs would make for easier review.

But looks good! None of my inline comments are blockers.

@mattculler I added the spec in the PR description. The PR is indeed doing both but the actual spec implementation is very very small here (this is just about setting constraints on base and build_base) so I was not sure it was worth splitting.

tigarmo

Thanks! I mostly reviewed the contents for the imagecraft directory, let me know if there's anything important outside of it that I should look at (I assumed most of it is the standard starbase/starflow stuff)

imagecraft/cli.py

imagecraft/models/project.py

imagecraft/services/lifecycle.py

imagecraft/services/pack.py

upils · 2025-01-16T15:31:45Z

Thanks! I mostly reviewed the contents for the imagecraft directory, let me know if there's anything important outside of it that I should look at (I assumed most of it is the standard starbase/starflow stuff)

@tigarmo Thanks! I am also interested in a review of the GH workflow changes and the spread-related ones (I previously had tests on various systems for amd64/arm64 that I removed for now since the tool is not doing much).

mattculler

Looking good

medubelko

@upils I fixed the build and did some reorganisation of the docs so they align with current practices. Everything looks good from my end.

upils · 2025-01-17T10:24:15Z

@upils I fixed the build and did some reorganisation of the docs so they align with current practices. Everything looks good from my end.

Great, thanks!

@medubelko I also changed the configuration on RTD to use .readthedocs.yaml and not docs/.readthedocs.yaml. I am not sure if that was something I changed at some point in the past or if this is the default configuration though. Do you know?

The build is now looking for docs/.sphinx/build_requirements.py (which is missing). Do you want to dig and fix this or can we do that in a following PR?

Signed-off-by: Paul Mars <[email protected]>

…ork on older release Signed-off-by: Paul Mars <[email protected]>

Signed-off-by: Paul Mars <[email protected]>

Applying canonical/starbase@03aff98 Signed-off-by: Paul Mars <[email protected]>

Signed-off-by: Paul Mars <[email protected]>

Signed-off-by: Sergio Schvezov <[email protected]>

lengau · 2025-01-18T00:06:19Z

.readthedocs.yaml

I can't see why the RTD builds are failing, but they're currently failing.

warnings,

WARNING: The local Git repository could not be found.

Seems like craft-cli had these once upon a time (not longer than a month ago), so I decided to allow them for now https://app.readthedocs.com/projects/canonical-craft-cli/builds/2700288/#30829517--59

lengau · 2025-01-18T00:07:18Z

HACKING.rst

I know this is different from what we have in starbase, but I think @medubelko wants us to move over to CONTRIBUTING.rst so maybe we can do that here?

CONTRIBUTING is preferred, but I don't think we should tackle that right now.

Signed-off-by: Sergio Schvezov <[email protected]>

upils self-assigned this Jan 8, 2025

upils force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch 5 times, most recently from f0af6fd to 255adcc Compare January 10, 2025 15:13

upils requested review from tigarmo, lengau, medubelko and mattculler January 13, 2025 07:32

upils marked this pull request as ready for review January 13, 2025 07:34

lengau reviewed Jan 13, 2025

View reviewed changes

.pre-commit-config.yaml Outdated Show resolved Hide resolved

lengau reviewed Jan 13, 2025

View reviewed changes

common.mk Show resolved Hide resolved

lengau previously approved these changes Jan 13, 2025

View reviewed changes

upils dismissed lengau’s stale review via 608ca43 January 14, 2025 08:15

lengau previously approved these changes Jan 14, 2025

View reviewed changes

upils dismissed lengau’s stale review via 7388f44 January 15, 2025 07:34

upils force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch from 7388f44 to cedb44f Compare January 15, 2025 07:39

mattculler previously approved these changes Jan 16, 2025

View reviewed changes

upils dismissed mattculler’s stale review via 030383c January 16, 2025 08:51

upils force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch from cedb44f to 030383c Compare January 16, 2025 08:51

tigarmo reviewed Jan 16, 2025

View reviewed changes

imagecraft/cli.py Show resolved Hide resolved

imagecraft/models/project.py Show resolved Hide resolved

imagecraft/services/lifecycle.py Show resolved Hide resolved

imagecraft/services/pack.py Show resolved Hide resolved

tigarmo previously approved these changes Jan 16, 2025

View reviewed changes

mattculler previously approved these changes Jan 16, 2025

View reviewed changes

medubelko dismissed stale reviews from mattculler and tigarmo via 9a966bf January 17, 2025 01:46

medubelko previously approved these changes Jan 17, 2025

View reviewed changes

upils added 12 commits January 17, 2025 11:30

ci: only run tests on noble and onward

3a96d71

Signed-off-by: Paul Mars <[email protected]>

chore(deps): add libgit2-dev to enable building pygit2

3f1897e

Signed-off-by: Paul Mars <[email protected]>

chore: Also pack the snap when calling make pack

19114b3

Signed-off-by: Paul Mars <[email protected]>

chore(spread): linting

f1847fe

Signed-off-by: Paul Mars <[email protected]>

test(spread): remove tests on previous implementation

de00ec2

Signed-off-by: Paul Mars <[email protected]>

test(spread): install make in the backend

5b9b264

Signed-off-by: Paul Mars <[email protected]>

ci: List the wanted extras to include in the python scan

2ace38b

Signed-off-by: Paul Mars <[email protected]>

ci(spread): Always use the snap built at the previous job

6ccf76d

Signed-off-by: Paul Mars <[email protected]>

ci(spread): Do not test on arm

ad9d92f

Signed-off-by: Paul Mars <[email protected]>

build: Make sure symlinks to python binaries are present to make it w…

47c31a1

…ork on older release Signed-off-by: Paul Mars <[email protected]>

build(deps): update pre-commit hook astral-sh/ruff-pre-commit to v0.9.1

3699380

Signed-off-by: Paul Mars <[email protected]>

fix: freeze UV by default

7337ac0

Applying canonical/starbase@03aff98 Signed-off-by: Paul Mars <[email protected]>

upils dismissed medubelko’s stale review via 7337ac0 January 17, 2025 10:42

upils force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch from 9a966bf to 7337ac0 Compare January 17, 2025 10:42

tigarmo previously approved these changes Jan 17, 2025

View reviewed changes

fix(doc): Do not install extra stuff before building the doc with RTD

2e28a9e

Signed-off-by: Paul Mars <[email protected]>

upils dismissed tigarmo’s stale review via 2e28a9e January 17, 2025 16:12

upils and others added 3 commits January 17, 2025 17:14

fix(doc): Do not care about scripts when building with RTD

ac2316c

Signed-off-by: Paul Mars <[email protected]>

feat: minimal template for init

85ca047

Signed-off-by: Sergio Schvezov <[email protected]>

docs: ignore overlay command reference

0408e92

Signed-off-by: Sergio Schvezov <[email protected]>

sergiusens force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch from 2922f1a to 0408e92 Compare January 17, 2025 21:40

lengau reviewed Jan 18, 2025

View reviewed changes

lengau approved these changes Jan 18, 2025

View reviewed changes

sergiusens force-pushed the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch 2 times, most recently from 9a3310a to 0408e92 Compare January 18, 2025 00:30

docs: allow readthedocs warnings

a55972e

Signed-off-by: Sergio Schvezov <[email protected]>

sergiusens approved these changes Jan 18, 2025

View reviewed changes

upils merged commit 9bdd6f0 into main Jan 20, 2025
5 checks passed

upils deleted the work/CRAFT-3862-Implement-the-initial-Imagecraft-spec branch January 20, 2025 07:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

work/CRAFT-3862-Implement-the-initial-Imagecraft-spec #70

work/CRAFT-3862-Implement-the-initial-Imagecraft-spec #70

upils commented Jan 8, 2025 •

edited

Loading

mattculler left a comment

upils commented Jan 16, 2025

tigarmo left a comment

upils commented Jan 16, 2025 •

edited

Loading

mattculler left a comment

medubelko left a comment

upils commented Jan 17, 2025 •

edited

Loading

lengau Jan 18, 2025

sergiusens Jan 18, 2025

lengau Jan 18, 2025

medubelko Jan 18, 2025

work/CRAFT-3862-Implement-the-initial-Imagecraft-spec #70

work/CRAFT-3862-Implement-the-initial-Imagecraft-spec #70

Conversation

upils commented Jan 8, 2025 • edited Loading

mattculler left a comment

Choose a reason for hiding this comment

upils commented Jan 16, 2025

tigarmo left a comment

Choose a reason for hiding this comment

upils commented Jan 16, 2025 • edited Loading

mattculler left a comment

Choose a reason for hiding this comment

medubelko left a comment

Choose a reason for hiding this comment

upils commented Jan 17, 2025 • edited Loading

lengau Jan 18, 2025

Choose a reason for hiding this comment

sergiusens Jan 18, 2025

Choose a reason for hiding this comment

lengau Jan 18, 2025

Choose a reason for hiding this comment

medubelko Jan 18, 2025

Choose a reason for hiding this comment

upils commented Jan 8, 2025 •

edited

Loading

upils commented Jan 16, 2025 •

edited

Loading

upils commented Jan 17, 2025 •

edited

Loading