Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Rework listing of Notifier bridges#15197

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
OskarStark wants to merge2 commits intosymfony:5.xfromOskarStark:rework-notifiers

Conversation

@OskarStark
Copy link
Contributor

@OskarStarkOskarStark commentedApr 7, 2021
edited
Loading

This is a first idea to rearrange the notifier bridges, because its a lot of work to change the table all the time, the diff is unclear and noisy.

Current state:
CleanShot 2021-04-07 at 09 34 38

If we found a decision, I would propose to create separate files for each notifier which could be included and sorted alphabetically easily.

Option 1 keeps the same depth and uses a definition list for the bridges.
Option 2 changes the depth and uses dedicated sub headings for the name (which is a plus, because you can link to a bridge directly)

My preferred option based on the facts would be *Option 2, but I would love to see it in action with a SymfonyCloud environment 😃 cc@javiereguiluz can you do this manually?

@javiereguiluzjaviereguiluz changed the titleRework listing of Notifier brdigesRework listing of Notifier bridgesApr 7, 2021
@javiereguiluz
Copy link
Member

I'm a big fan of NOT using tables in RST because they cause many problems when the docs are alive and continuously updated.

However, here we're using the compact RST table syntax, which doesn't add dashes in all rows and columns, so maintenance should be nice (I've suffered myself some conflicts when merging things, but it's easy to solve them because of this compact table syntax).

The problem I see(but only in this case, in others it would be nice to replace a table by several subsections) is that now we can quickly overview all available services in a small table ... but if we turn this into subsections, the information is now spread in a very very long document, because each section needs lots of space.

An additional issue is that these subsections wouldn't appear in the Table of Contents of the page, so you can't easily navigate through them.

So, it's tricky (RST tables always are) but I'd say there are more reasons to keep this doc "as is" instead of changing it.

@OskarStark
Copy link
ContributorAuthor

Thank you for your feedback Javier, much appreciated.

I came to this proposal based on#15193 which was a lot of work just for this one new bridge and the diff is .... 👎

We can also think about using Option 1 or option 2 on a dedicated page, I know we want to go away from many page more to "complete" pages, but there is no silver bullet IMHO

@wouterj
Copy link
Member

The diff can be fixed by ignoring whitespace:https://github.com/symfony/symfony-docs/pull/15193/files?w=1

I agree with@javiereguiluz, tables just make sense here. The line wrapping is a bit suboptimal, but maybe we can remove a column?

@OskarStark
Copy link
ContributorAuthor

The diff can be fixed by ignoring whitespace:https://github.com/symfony/symfony-docs/pull/15193/files?w=1

Sure, nonetheless I as a contributor needed to touch and move a lot of whitespaces to get a valid table just to add one line.

The line wrapping is a bit suboptimal, but maybe we can remove a column?

I think we will end up all the time with moving spaces around, but OK, lets close then

@OskarStarkOskarStark deleted the rework-notifiers branchApril 7, 2021 10:25
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wouterjwouterjAwaiting requested review from wouterj

@javiereguiluzjaviereguiluzAwaiting requested review from javiereguiluz

@weaverryanweaverryanAwaiting requested review from weaverryan

Assignees

@OskarStarkOskarStark

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@OskarStark@javiereguiluz@wouterj@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp