8000 Revamping Multiple Kernels documentation by yceruto · Pull Request #18186 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

Revamping Multiple Kernels documentation #18186

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

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 4, 2023
Merged

Revamping Multiple Kernels documentation #18186

merged 1 commit into from
May 4, 2023

Conversation

yceruto
Copy link
Member
@yceruto yceruto commented Apr 11, 2023

As we discussed on Slack, this approach will simplify and remove the boilerplate code while the result and goals are the same as before.

@carsonbot carsonbot added this to the 5.4 milestone Apr 11, 2023
@yceruto yceruto force-pushed the revamp_multiple_kernel branch from 8341d06 to 1d1658d Compare April 11, 2023 01:55
@yceruto
Copy link
Member Author
yceruto commented Apr 11, 2023

Lint failures are false positive in this case.

94noni
94noni reviewed Apr 11, 2023
View reviewed changes
94noni
94noni reviewed Apr 11, 2023
View reviewed changes
94noni
94noni approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor
@94noni 94noni left a comment 8000

Choose a reason for hiding this comment

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

really useful rewrite :)

@yceruto
Copy link
Member Author
yceruto commented Apr 11, 2023

Thanks for your feedback @94noni! I will take care of them soon.

@yceruto yceruto force-pushed the revamp_multiple_kernel branch from 838899f to 49b969a Compare April 18, 2023 14:43
@javiereguiluz javiereguiluz force-pushed the revamp_multiple_kernel branch from 84c941b to f1698a4 Compare May 4, 2023 14:21
@javiereguiluz javiereguiluz merged commit 9849165 into symfony:5.4 May 4, 2023
@javiereguiluz
Copy link
Member
javiereguiluz commented May 4, 2023 •
edited
Loading

Yonel, what an impressive contribution 🙇🙇

This is really useful and your explanations were very detailed and practical. It's amazing!

While merging I did some tweaks (see b3bee8e). Even if it looks like it's a lot of changes, in reality it's not:

  • I wrapped the long lines to match our line-length limit
  • I reworded the "we", "let's" and "us" expressions to "you" and "your", etc. In the Symfony Docs we never use "we", so I had to make this change to make the docs consistent with the other contents.

So, in practice, 99% of your original contribution remained 🙏 Thanks!

@yceruto yceruto deleted the revamp_multiple_kernel branch May 4, 2023 17:17
@yceruto
Copy link
Member Author
yceruto commented May 4, 2023

Thank you Javier for the tweaks! I will take them into account next time 👍

@yceruto
Copy link
Member Author
yceruto commented May 4, 2023

Minor tweaks about a visual issue in the directory structure #18279

@alamirault
Copy link
Contributor
alamirault commented May 7, 2023 •
edited
Loading

Doctor-rst complains about name apps and want applications indeed
https://github.com/symfony/symfony-docs/actions/runs/4883935071/jobs/8715959800

Do you think we can update docs with this naming (PR #18295) ? or we must add it to whitelist ?

@alamirault alamirault mentioned this pull request May 7, 2023
Closed
javiereguiluz added a commit that referenced this pull request May 9, 2023
@javiereguiluz
minor #18295 [Fix pipeline] multiple_kernels.rst must respect doctor-…
e562839 
…rst (alamirault)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Fix pipeline] multiple_kernels.rst must respect doctor-rst

#18186 (comment)

`apps` -> `applications`

`ContainerConfigurator $container` -> `ContainerConfigurator $containerConfigurator`

Commits
-------

07f6609 [Fix pipeline] multiple_kernels.rst must respect doctor-rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@94noni 94noni 94noni approved these changes

Assignees
No one assigned
Labels
Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
5 participants
@yceruto @javiereguiluz @alamirault @94noni @carsonbot
0