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: update usage/channels #3351

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

docs: update usage/channels #3351

wants to merge 3 commits into from

Conversation

JacobCoffee
Copy link
Member

Description

Closes

@JacobCoffee JacobCoffee added the no changelog Do not add this entry to the changelog label Apr 9, 2024
@JacobCoffee JacobCoffee requested review from a team as code owners April 9, 2024 03:59
@github-actions github-actions bot added area/docs This PR involves changes to the documentation size: small type/docs pr/internal labels Apr 9, 2024
@JacobCoffee
Copy link
Member Author

I recommend reviewing with whitespace changes off because the diff is stupid

docs/usage/channels.rst Show resolved Hide resolved
docs/usage/channels.rst Outdated Show resolved Hide resolved
docs/usage/channels.rst Outdated Show resolved Hide resolved
@JacobCoffee
Copy link
Member Author

ready

Copy link

sonarcloud bot commented Apr 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

github-actions bot commented Apr 9, 2024

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3351

@@ -11,115 +11,143 @@ Channels provide:

1. Independent :term:`broker` backends, optionally handling inter-process communication
and data persistence on demand
2. "Channel" based subscription management
2. "Channel" based :term:`subscription` management
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems the glossary section needs to be expanded for the term links to work. Can that be expanded by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can but if it I think i'd prefer it at the bottom of the page since its chunky


Broker --> Backend_1(Backend)
Broker --> Backend_2(Backend)
.. dropdown:: Click to expand flowcharts
Copy link
Contributor

Choose a reason for hiding this comment

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

image


Broker --> Backend_1(Backend)
Broker --> Backend_2(Backend)
.. dropdown:: Click to expand flowcharts
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually necessary? We already have the side-nav where you can easily skip the section if you're not interested in that.


.. note::
While calling :meth:`publish <ChannelsPlugin.publish>` does not guarantee the
.. note:: While calling :meth:`~ChannelsPlugin.publish` does not guarantee the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. note:: While calling :meth:`~ChannelsPlugin.publish` does not guarantee the
.. note::
While calling :meth:`~ChannelsPlugin.publish` does not guarantee the

The first line shouldn't be the title :)

.. seealso::

* `Managing backpressure`_
Read more: `Managing backpressure`_
Copy link
Member

Choose a reason for hiding this comment

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

Why no admonition? :(

Copy link
Member Author

Choose a reason for hiding this comment

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

It was nested inside of another one 😬


.. important::
The :term:`events <event>` in the :term:`event streams <event stream>` are always
.. important:: The :term:`events <event>` in the :term:`event streams <event stream>` are always
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. important:: The :term:`events <event>` in the :term:`event streams <event stream>` are always
.. important::
The :term:`events <event>` in the :term:`event streams <event stream>` are always

The publication of the history happens sequentially, one channel and one
event at a time. This is done to ensure the correct ordering of events and to avoid
filling up a subscriber's backlog, which would result in dropped history entries. Should the
.. note:: The publication of the :term:`history` happens sequentially, one :term:`channel` and one
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. note:: The publication of the :term:`history` happens sequentially, one :term:`channel` and one
.. note::
The publication of the :term:`history` happens sequentially, one :term:`channel` and one

Screenshot from 2024-04-09 18-16-24

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems to be a spacing issue it shouldnt do this, everywhere else i've done this it is proper. Will fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs This PR involves changes to the documentation no changelog Do not add this entry to the changelog pr/internal size: small type/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants