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: add top-level intro and module-level intros #1320

Merged
merged 32 commits into from
Sep 20, 2024

Conversation

dimaqq
Copy link
Contributor

@dimaqq dimaqq commented Aug 16, 2024

Docs: Ops: Add top-level intro, clean up ops.module intro in light of that, flesh out ops.pebble and ops.testing intros a bit more

https://warthogs.atlassian.net/browse/CHARMTECH-183

  • "home page" library reference
  • ops module
  • ops.main entry point
  • ops.pebble module
  • ops.testing module

@dimaqq
Copy link
Contributor Author

dimaqq commented Sep 17, 2024

Also fixes #1263

@dimaqq
Copy link
Contributor Author

dimaqq commented Sep 17, 2024

@tmihoc the PR is mostly ready for your review.
(except the OPT1/OPT2 to be discussed with the team)

docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Show resolved Hide resolved
docs/index.rst Show resolved Hide resolved
ops/__init__.py Show resolved Hide resolved
ops/__init__.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
ops/__init__.py Outdated Show resolved Hide resolved
@dimaqq dimaqq changed the title docs: wip docs: add top-level intro and module-level intros Sep 18, 2024
ops/framework.py Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
ops/__init__.py Show resolved Hide resolved
ops/__init__.py Outdated Show resolved Hide resolved
ops/pebble.py Outdated Show resolved Hide resolved
ops/testing.py Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
ops/framework.py Show resolved Hide resolved
Copy link
Collaborator

@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.

Looks good, just two more minor suggestions, but I'm happy to merge. Thanks!

ops/__init__.py Outdated Show resolved Hide resolved
ops/pebble.py Outdated Show resolved Hide resolved
ops/testing.py Show resolved Hide resolved
@dimaqq dimaqq merged commit 454e197 into canonical:main Sep 20, 2024
29 checks passed
@dimaqq dimaqq deleted the docs-main-page branch September 20, 2024 00:28
@tmihoc
Copy link
Member

tmihoc commented Sep 20, 2024

I was looking at the published result and just noticed that, while the top-level intro starts with ops.main entry point, the Navigation / order of the sections starts with ops module, and only then the entry point. I'm not sure which one is better but I feel we should be consistent. PS Judging by what we had in the Charm SDK docs, we should start with the entrypoint. Basically, the minute people write import ops at the top, they should follow up with the main bit, which should anticipate the charm class argument:

ops.main()
The Ops library provides a top-level main method. This method should be the main entrypoint for your charm, like so:

import ops

# ... charm code

if __name__ == "__main__":
  ops.main(HelloCharm)
The method takes a charm class (see the next section) as an argument, and is used to initialise the charm code when the charm is invoked and ensure that events are dispatched to the charm in response to controller-emitted events.

(To clarify, I'm not suggesting we change anything about the text on https://ops.readthedocs.io/en/latest/#ops.Framework -- just the order of the ops.main entrypoint vs. ops module.)

Sorry I only noticed now.

@tonyandrewmeyer
Copy link
Contributor

I was looking at the published result and just noticed that, while the top-level intro starts with ops.main entry point, the Navigation / order of the sections starts with ops module, and only then the entry point. I'm not sure which one is better but I feel we should be consistent.

@benhoyt noticed this in #1381 as well, so I've adjusted it there. We went with ops first and then ops.main, because most of the time when people come to the reference docs, they'll be wanting something in ops. Once you know how to use ops.main (which you probably got from the tutorial, and then the code from charmcraft init), you'll almost never want to look at its reference docs.

@tmihoc
Copy link
Member

tmihoc commented Sep 26, 2024

Once you know how to use ops.main (which you probably got from the tutorial, and then the code from charmcraft init), you'll almost never want to look at its reference docs.

While that's true, in principle, content should always have an obvious order, either natural (numerical, for tightly ordered things, or lifecycle, for more loosely ordered things) or, if that's not possible alphabetical. Here I'd use natural order at the high levels and alphabetical within. So,

  • ops (natural order:)
    • ops.main entrypoint
    • ops.main (alphabetical order:)
      • ActionEvent
      • ...
    • ops.pebble
    • ops.testing
    • ops.testing.harness (legacy)

So that's the order I would use.

That is, imo, the fact that ops.main entrypoint is something you only use once shouldn't affect placement.

tonyandrewmeyer pushed a commit to tonyandrewmeyer/operator that referenced this pull request Oct 4, 2024
Docs: Ops: Add top-level intro, clean up ops.module intro in light of
that, flesh out ops.pebble and ops.testing intros a bit more

https://warthogs.atlassian.net/browse/CHARMTECH-183

- [x] "home page" library reference
- [x] ops module
- [x] ops.main entry point
- [x] ops.pebble module
- [x] ops.testing module
tonyandrewmeyer pushed a commit to tonyandrewmeyer/operator that referenced this pull request Oct 4, 2024
Docs: Ops: Add top-level intro, clean up ops.module intro in light of
that, flesh out ops.pebble and ops.testing intros a bit more

https://warthogs.atlassian.net/browse/CHARMTECH-183

- [x] "home page" library reference
- [x] ops module
- [x] ops.main entry point
- [x] ops.pebble module
- [x] ops.testing module
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.

4 participants