Skip to content

[Messenger] Replace tables by definition lists #20326

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

Closed
wants to merge 1 commit into from

Conversation

javiereguiluz
Copy link
Member

As a general rule, we should never use tables for list of options with long descriptions. The tables look ugly and the option names are unreadable:

image

In this PR, I propose to replace all tables with definition lists, which we use in all the other docs.

@javiereguiluz javiereguiluz changed the base branch from 6.4 to 7.2 October 14, 2024 17:43
@javiereguiluz javiereguiluz changed the base branch from 7.2 to 6.4 October 14, 2024 17:46
@javiereguiluz javiereguiluz changed the base branch from 6.4 to 7.2 October 14, 2024 17:47
Comment on lines +1735 to +1736
``sentinel_master``
String, if null or empty Sentinel support is disabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the redis_sentiel alias has been removed, was this intentional ?

@MrYamous
Copy link
Contributor

MrYamous commented Oct 15, 2024

That makes me think, what about dropping documentation for third-party transport configuration as done for Mailer and Notifier in favor of readme for each bridge ?

@javiereguiluz
Copy link
Member Author

Closing in favor of #20330 because this one contains a rebasing issue that I can't fix.

@javiereguiluz
Copy link
Member Author

@MrYamous I wouldn't do that because it's very cumbersome to readers. Even if it's a bit long, having all options in the same article is super convenient.

In Mailers, Notifiers, etc. we can't do that because there are tens and tens of integrations and they change frequently, and nobody in the Core Team uses most of them, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants