search

canonical / operator

Pure Python framework for writing Juju charms
Apache License 2.0
247 stars 119 forks source link

docs: move Pebble to a separate page #1392

Closed tonyandrewmeyer closed 1 month ago

tonyandrewmeyer commented 1 month ago

Pros:

Cons:

tonyandrewmeyer commented 1 month ago

@tmihoc feel free to provide an opinion here too, if you have time & want to :)

benhoyt commented 1 month ago

@tonyandrewmeyer RTD docs are failing to build. I'll wait to review till you've fixed that so I can compare with https://ops.readthedocs.io/en/latest/

tonyandrewmeyer commented 1 month ago

@tonyandrewmeyer RTD docs are failing to build. I'll wait to review till you've fixed that so I can compare with https://ops.readthedocs.io/en/latest/

Huh, they built without error locally, and I didn't get an email about the failure; weird. Thanks for letting me know - I'll figure out what's going on.

tonyandrewmeyer commented 1 month ago

Oops, this might explain it :blush: 

tameyer@tam-canoncial-1:~/code/operator$ git status
On branch pebble-doc-separate-page
Your branch is up to date with 'origin/pebble-doc-separate-page'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
    docs/pebble.rst

nothing added to commit but untracked files present (use "git add" to track)
tonyandrewmeyer commented 1 month ago

@benhoyt it should be ready now, sorry.

tmihoc commented 1 month ago

@tmihoc feel free to provide an opinion here too, if you have time & want to :)

Giving that the Pebble module is indeed quite independent, I think the move makes sense.

Unrelatedly: Looking at https://ops.readthedocs.io/en/latest/index.html, the Navigation, there are two things that I'd do differently:

(1) nitpick: Unit testing (was: Scenario): The "was" there is not great. I'd rephrase as "Unit testing (previously, "Scenario"). (2) a bigger concern: API reference, Pebble client, Unit testing (was: Scenario), Harness (legacy unit testing): The titles don't help me see the pieces and the relationships between them clearly. API reference = for what? How does it connect to the Pebble client or the Unit testing or the Harness bits? I don't have a clear suggestion but I think we should take a moment to think about this.

tonyandrewmeyer commented 1 month ago

(1) nitpick: Unit testing (was: Scenario): The "was" there is not great. I'd rephrase as "Unit testing (previously, "Scenario").

This is intended to only be there for a few months, for what it's worth, to help out all the people that know of Scenario understand that it's the same thing. It could be "previously", although it does then wrap to two lines, which is not ideal.

(2) a bigger concern: API reference, Pebble client, Unit testing (was: Scenario), Harness (legacy unit testing): The titles don't help me see the pieces and the relationships between them clearly. API reference = for what?

It could be "Ops API reference" I guess? If you look down from the top of the page, it says "The Ops Library", then "The ops library documentation" and then "API reference".

How does it connect to the Pebble client or the Unit testing or the Harness bits?

I think Pebble is tricky because ops wraps the Pebble client in the Container object, so for the most part charmers should not need to worry about what's in "Pebble client" because they should be using what's in the "Container" section of "API reference". However, Container leaks a bunch of ops.pebble stuff (error classes, layers, etc) so it's not entirely wrapped, and charmers do need to go to "Pebble client" for those (and "client" in that case is somewhat wrong).

We could introduce a layer, like:

Would that help?

It could be by namespace, but we decided to merge the two APIs in ops.testing, so that would be:

Or something like that.

I don't have a clear suggestion but I think we should take a moment to think about this.

Sure :)

tmihoc commented 1 month ago

It could be by namespace, but we decided to merge the two APIs in ops.testing, so that would be:

  • ops
  • ops.pebble
  • ops.testing
  • ops.testing (legacy API)

This makes the most sense to me and I just typed something like this too on your other PR: https://github.com/canonical/operator/pull/1320#issuecomment-2376265039

With a small difference:

Where ops is basically the top-most layer and may not need a separate entry in Navigation -- so the Navigation can skip to the component items. I wouldn't bother to group them into Development and Testing -- the extra layers imo bury content rather than helping surface it. We could group them like that in the top-level intro, if at all. E.g.,

The ops library provides:

  • development tools:
  • ops.main entrypoint
  • ops.main
  • ops.pebble
  • testing tools:
  • ...