-
Notifications
You must be signed in to change notification settings - Fork 55
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(snapcraft): aggregate lifecycle docs into an explanation #163
docs(snapcraft): aggregate lifecycle docs into an explanation #163
Conversation
2cfb4e8
to
d8ae70b
Compare
f1b0922
to
9fc34e5
Compare
Streamline documentation and add links to the reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this -- good work! Left some comments, mostly high-level thoughts (those ending with '?'). Others are, I think, more necessary changes.
Thank you @dilyn-corner and @medubelko for your precious comments! I will take all that into account and create new commits very soon! |
Hi @dilyn-corner and @medubelko, I reviewed your comments except for the support build-aux/snap comment. After talking with Michael we decided that I could try to improve the lifecycle explanation even if it's out of the scope of the initial issue. Two topics remain on my side:
Is there anything else that you think could be missing from the lifecycle explanation? Thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added information about permissions in the prime step. I think the users would benefit from having a link to more info on this topic but I don't think the lifecycle explanation is the place to do that.
Agreed; I also haven't ever really seen documentation surrounding this, which gives me the impression that it's more of a general "craft" thing than a snap thing (is it for rocks? charms?).
Can I link the readthedocs page and maybe in another issue a snapcraft.io page on permissions could be created?
Ultimately I think it may fall to @mr-cal's judgement; I've never seen a snap use this syntax.
I would like to add a similar processing diagram. Because it will need to be redone from sratch, I wanted your approval before working on it.
I think this would be a very powerful explanatory tool, but I also don't think it should block merging these excellent changes.
Is there anything else that you think could be missing from the lifecycle explanation?
It may be useful to add a link to organize
's definition or use similarly to how it's done for the stage
keyword; I think at the very least a link to here (and that page having anchors would be super useful...).
142419e
to
ca62923
Compare
Hi @dilyn-corner and @medubelko ,
|
Hi @dilyn-corner and @medubelko, I believe the diagram could benefit from a written text to enhance accessibility. This could be done in another issue to ensure it doesn't block the PR. |
This looks fantastic, @Sophie-Pages! I think there needs to be an "N" on the right arrow flowing from the "Step already ran?" node. Beyond that, I think this is a fantastic set of changes to merge with some light TODOs for future expansion. @medubelko, thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sophie-Pages Thanks for putting this together!
The comments that start with "For later" are notes for porting and migration. It will be a two step process where we move this to Discourse first, and then later perform even more of a consolidation before migration to RTD later in the spring.
There's one small comment about Snapcraft 6 that you could look into, and I've pinged the developer to give the steps in the explanation a once-over.
I'm fine with using this work as-is. I'll make tweaks to the language for style, but those don't need to block this PR.
Thanks again!
@@ -0,0 +1,155 @@ | |||
# Parts lifecycle | |||
|
|||
Parts, alongside [plugins](/t/snapcraft-plugins/4284), are a key component in any [Snapcraft](/t/snapcraft-overview/8940) project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For later:
Explanations are like dissertations. For this one, if we were to expand on the title, it would be something like "About the parts lifecycle".
To serve the goals of an explanation, the intro should be the thesis statement of the whole page, and answer:
- What is this thing?
- Why does it exist, and why does it matter?
- What is the rest of this page going to explore about this thing?
|
||
Parts, alongside [plugins](/t/snapcraft-plugins/4284), are a key component in any [Snapcraft](/t/snapcraft-overview/8940) project. | ||
|
||
See [Adding parts](/t/adding-parts/11473) for a general overview of what parts are and how to use them, [Scriptlets](https://forum.snapcraft.io/t/scriptlets/4892) for details on how they can be scripted outside of _snapcraft.yaml_, and <!--TO DO: Path to be added when the page is created--> [Parts lifecycle]() reference for the summarised information of this page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For later:
Let's move these links to the end. They're not a priority in the intro.
|
||
Note that each command also executes the previous lifecycle steps, so `snapcraft` executes all the lifecycle steps chained together. | ||
|
||
To access the part environment at any stage, add the `--shell` argument. For example, `snapcraft prime --shell` will run up to the *prime* step and open a shell. See [Iterating over a build](/t/iterating-over-a-build/12143) for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For later:
We should shift this language to be more explanatory rather than guiding.
When running through its lifecycle steps, a part will use different working directories. The directories' names closely follow the steps' names. | ||
|
||
| Environment variable | Directory | Purpose | | ||
|--|--|--| | ||
| `CRAFT_PART_SRC` | **`parts/<part-name>/src`** | the location of the source after the *pull* step | | ||
| `CRAFT_PART_BUILD` | **`parts/<part-name>/build`** | the working directory during the *build* step | | ||
| `CRAFT_PART_INSTALL`| **`parts/<part-name>/install`** | contains the results of the *build* step and the stage packages. It is also the directory where the `organize` event renames the built files | | ||
| `CRAFT_STAGE` | **`stage`** | shared by all parts, this directory contains the contents of each part's `CRAFT_PART_INSTALL` after the *stage* step. It can contain development libraries, headers, and other components (e.g. pkgconfig files) that need to be accessible from other parts | | ||
| `CRAFT_PRIME` | **`prime`** | shared by all parts, this directory holds the final components after the *prime* step | | ||
| `CRAFT_PROJECT_DIR` | path to the current project's subtree in the filesystem. | path to the resulting snap after the *pack* step | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For later:
A potential candidate for a separate reference page covering all tabular data and lists related to parts.
|
||
Please note that directories are specified by their environment variables: | ||
- **Snapcraft 7+**: environment variables start with `CRAFT_` | ||
- **Snapcraft 6, and earlier**: variables start with `SNAPCRAFT_` but are otherwise identical |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove any information about Snapcraft 6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For later:
The chart assumes prior knowledge about the system. Users would be well-served if we provided more context through extra labels. All of the text could be also simplified and clarified.
This chart is from Craft Parts, and originally from a spec. It has some weaknesses, but it's fine to leave it in as-is for now.
Agreed! I think it's nearly ready for prime time, pending a look from Cal.
It's fine as-is – no need to sweat about the smaller details, since after I've ported this to Discourse I'll be migrating it to RTD. |
Hi @medubelko, Thank you for your comments, they were really helpful. I took this chance to improve the explanation and to reduce the smaller details you will need to go through. I didn't remove the reference, I'll let you decide what you want to do with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, @Sophie-Pages! This is very comprehensive and readable.
I have a few small nitpicks and one comment about CRAFT_PROJECT_DIR
.
@mr-cal I can't find any info on |
@Sophie-Pages If you could incorporate Cal's small suggestions, I'll be happy to mark this done and begin the work of bringing it home. :) |
Thank you @mr-cal and @medubelko! I incorporated Cal's suggestions. I think we are good to go to the next step! |
|
First step into moving the Parts lifecycle into Snapcraft's reference: create the reference.
It is missing the additional work and redirect links on the other Snapcraft pages.