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

Add a diagram for parts.rst #428

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

Conversation

sed-i
Copy link

@sed-i sed-i commented Apr 28, 2023

This PR adds a diagram to the "parts" doc.
Edit a live copy of the diagram here.

@sed-i
Copy link
Author

sed-i commented Apr 29, 2023

@cmatsuoka @cjdcordeiro @tigarmo wdyt about this as a quick-glance reference?
Surely still needs to be polished.

@cjdcordeiro
Copy link

I think mermaid would be a nice addition to the docs, to avoid static content

@@ -31,6 +31,31 @@ describe a build process typically accept specifications of parts in YAML format
mostly-declarative format. Libraries that use parts may use the underlying
data structures to describe them.

.. mermaid ::
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this high-level diagram but I'm wondering about its position here; there is a Lifecycle-specific page with similar-but-different diagram (here), but I don't know if things are organized as well as they could be.
@dboddie can you take a look at this page, and the Lifecycle page, and give your opinion here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good start but I'll need to fix up the configuration and modify the diagram to fit it onto the page. It's a bit wide at the moment.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @dboddie.
If the width is not autoscaled then perhaps graph TD would look better than graph LR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll give it a try.

Copy link

@ca-scribner ca-scribner May 16, 2023

Choose a reason for hiding this comment

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

EDIT: Whoops, I originally comment on an old version of the figure. Comment is updated now

This diagram is great and with a few minor tweaks I think it'll be fantastic in the docs.

I like that the nodes are now $CRAFT_PART_INSTALL, etc. That imo makes it clear they represent the directories. maybe that makes the nodes a bit odd to read at first, but if there's some text explaining that above the diagram it'll be fine I think

For the arrows, again some text explaining these are state transitions would be helpful. Linking them directly to commands (the build transition corresponds to *craft build, etc) would be even better.

I wonder if we can clarify the build transition. iiuc, build gets a copy of $CRAFT_PART_SRC to work on in $CRAFT_PART_BUILD, and then build is responsible for putting anything that'll go to organize/stage into CRAFT_PART_INSTALL? Does the user have any control over the copying of $CRAFT_PART_SRC to $CRAFT_PART_BUILD? The diagram makes me think it does, but I don't think that's correct (if I do override-build. For example, if you do override-build, you still get a populated $CRAFT_PART_SRC dir to start with (at least, I did when overriding the go plugin). Maybe there's a different way to represent this. Like...

...
$CRAFT_PROJECT_DIR 
--->|pull| $CRAFT_PART_SRC
--->|preprocessing for build| $CRAFT_PART_BUILD
--->|build| $CRAFT_PART_INSTALL
--->|stage| $CRAFT_STAGE
--->|prime| $CRAFT_PRIME
...

?

docs/explanation/parts.rst Outdated Show resolved Hide resolved
@cmatsuoka cmatsuoka added the Documentation Improvements or additions to documentation label May 10, 2023
@sed-i
Copy link
Author

sed-i commented Nov 3, 2023

Hi @tigarmo, @cmatsuoka, @cjdcordeiro,
If you still think this diagram is worthwhile to have, I can convert it to drawio + export to png under docs/images.
Otherwise please feel free to close this PR.

@cjdcordeiro
Copy link

Hey,

in the meantime, this new image was drafted:
image

We have already used it in a couple of places and might consider cleaning it up to include in (some) docs. There's a big doc refactoring item coming up over the next few months, so I'd keep these images in our watchlist for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants