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

docs: how to use layers #548

Merged
merged 20 commits into from
Jan 24, 2025

Conversation

IronCore864
Copy link
Contributor

@IronCore864 IronCore864 commented Jan 7, 2025

Docs: How to use layers.

As previously discussed, the layers reference doc is removed and refactored into a how-to.

Preview: https://canonical-pebble--548.com.readthedocs.build/en/548/how-to/use-layers/

@IronCore864 IronCore864 requested a review from dwilding January 7, 2025 08:47
@IronCore864 IronCore864 marked this pull request as ready for review January 7, 2025 08:47
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
@dwilding
Copy link
Contributor

dwilding commented Jan 8, 2025

Thanks @IronCore864, this new doc is a helpful summary of how to use layers! I've added some suggestions and questions about specific parts.

One other observation - In the intro, the doc mentions three benefits:

  1. organize services into logical groups
  2. define one set of configurations per environment
  3. layered configurations can also accommodate imperative overrides

Of these, it seems that we only illustrate 1 and 3. Is that because 2 is pretty similar to 1 in approach?

I think it would be nice if we could match up each benefit with a corresponding section. For example, at the end of the first bullet, we could say:

For an example, see Use layers as logical groups.

And similarly for the other benefits.

We could either add a dedicated section to show how to define one set of configurations per environment. Or if it's really essentially the same as how to organize services into logical groups, at the end of benefit 2 we could say something like:

The way to achieve this in Pebble is the same as how to organize services into logical groups, with each environment set up as a separate group.

What do you think?

@IronCore864 IronCore864 requested a review from dwilding January 8, 2025 09:02
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

This is the first how-to of the open PRs that I'm reviewing, and unfortunately I think it needs some rework. That said, it's a good document to start a discussion about the style and content we're looking for with these how-tos, because I think it's the one that needs the most work -- at a glance, the others look significantly "closer" to me.

I'll schedule a conversation with you, David, and me for next week so we can get agreement on how-to style and content more generally, but for now, here are some of my specific concerns with this doc:

  1. It's too long overall.
  2. The intro takes too long to get to the point; we're not writing a blog post or a general DevOps book, but a how-to for a specific aspect of a specific product.
  3. The YAML examples given are long-ish, and it's unclear what the results of merging is. I think we need shorter, more concrete examples, and a better way to show the results of the combined plan.
  4. It uses lots of big words and a passive voice. For example, you currently have sentences like:

Without proper service orchestration tooling, managing dependencies and ensuring consistent behaviour across multiple services in multiple environments is complex and error-prone.

Noun phrases like "service orchestration tooling" are a mouthful, and there's just a lot of words here. Perhaps something more like:

Pebble layers make it easier to manage services in multiple environments.

However, I actually think the intro needs a complete rewrite to get straight to the point and to a concrete use case.

  1. The plan/layers are more than just "services" ("checks" and "log-targets" are combined the same way).
  2. I recommend focusing on more concrete examples, as well as a more concrete use for actual layering. The one we talk about a lot is a base layer in a container image (likely a Rock) with additional values being layered on by the user of that image (for example, a charm).

Again, let's meet up to discuss the general approach before going further with these.

docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
@dwilding
Copy link
Contributor

dwilding commented Jan 22, 2025

@IronCore864 I've finished reviewing, thanks!

One thing we'll need to decide with @benhoyt is around US spelling of words (not only applicable to this doc, but it's where I noticed it most obviously). In this doc we use "organized" and "centralized", but also "behaviour", which isn't consistent.

I'd be happy to use US English for this doc. That would mean changing "behaviour" to "behavior". We already aren't consistent in spelling across the Pebble docs (see this issue that I just created to track), so I don't think anything is lost by adopting US English here.

@IronCore864 IronCore864 requested a review from benhoyt January 22, 2025 03:37
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

I think this is quite good now, thanks. A few comments. In addition, I think it'd be good to add a short section about dynamically adding layers (with pebble add).

docs/how-to/forward-logs-to-loki.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
@IronCore864 IronCore864 requested a review from benhoyt January 23, 2025 09:49
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Very close now. Just a few minor changes suggested.

docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
@IronCore864 IronCore864 requested a review from benhoyt January 24, 2025 02:09
Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Approving -- let's just remove before:. Thanks!

docs/how-to/use-layers.md Outdated Show resolved Hide resolved
docs/how-to/use-layers.md Outdated Show resolved Hide resolved
@IronCore864 IronCore864 merged commit 6455ef9 into canonical:master Jan 24, 2025
18 checks passed
@IronCore864 IronCore864 deleted the docs-how-to-use-layers branch January 24, 2025 06:30
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.

3 participants