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

[Config] AddNodeDefinition::docUrl()#59762

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

Merged

Conversation

@alexandre-daubois
Copy link
Member

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
Issues-
LicenseMIT

Adding such information would allow extensions and bundles to provide even more info with a documentation "one click away".

The primary goal is to use this feature in conjunction with#58771, allowing to dump a@see https://symfony.com/doc/... right next to the configuration array shape.

@GromNaN
Copy link
Member

This can also be used in the Yaml JsonSchema:#59620

alexandre-daubois reacted with rocket emoji

@alexandre-dauboisalexandre-dauboisforce-pushed theconfig-documentation-uri branch 2 times, most recently fromacbda3f tob9240b9CompareFebruary 13, 2025 08:16
@alexandre-daubois
Copy link
MemberAuthor

Should we bump the minimum requirement forsymfony/config to^7.3 in bundles, or conditionally calldocumentationUri() on the root node?

@stof
Copy link
Member

I suggest bumping the min version instead of doing conditional calls. It makes the code simpler

alexandre-daubois, OskarStark, and GromNaN reacted with thumbs up emoji

@alexandre-dauboisalexandre-dauboisforce-pushed theconfig-documentation-uri branch 4 times, most recently from6d8ef52 tob2502a8CompareFebruary 13, 2025 17:53
@stof
Copy link
Member

You should probably also update the dumpers to add a comment with the documentation URL (when present), so thatdebug:config andconfig:dump-reference show it.

tacman reacted with thumbs up emoji

@alexandre-daubois
Copy link
MemberAuthor

Right, here's the result with the latest push:

Capture d’écran 2025-02-14 à 10 14 52Capture d’écran 2025-02-14 à 10 07 21

@alexandre-dauboisalexandre-dauboisforce-pushed theconfig-documentation-uri branch 2 times, most recently from3a00a22 toc7f73c8CompareFebruary 19, 2025 12:43
@alexandre-dauboisalexandre-daubois changed the title[Config] AddNodeDefinition::documentationUri()[Config] AddNodeDefinition::docUrl()Mar 25, 2025
publicfunctiondocUrl(string$uri, ?string$package =null):static
{
if ($package) {
preg_match('/^(\d+)\.(\d+)\.(\d+)/', InstalledVersions::getVersion($package) ??'',$m);
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

No$, because some packages may return a fourth number (in local,phpunit/phpunit returns9.6.99.0 for example on my computer)

"require": {
"php":">=8.2",
"ext-xml":"*",
"composer-runtime-api":">=2.1",
Copy link
Member

Choose a reason for hiding this comment

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

When installing the smallest possible Symfony app, we currently only require Symfony deps. So, that would be the first non-Symfony dependency that we require.

Choose a reason for hiding this comment

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

I had the same thought until I realized the framework bundle already has this dependency:
https://github.com/symfony/symfony/blob/7.3/src/Symfony/Bundle/FrameworkBundle/composer.json

@fabpot
Copy link
Member

Thank you@alexandre-daubois.

@fabpotfabpot merged commit2795d95 intosymfony:7.3Apr 4, 2025
9 of 11 checks passed
@fabpotfabpot mentioned this pull requestMay 2, 2025
fabpot added a commit that referenced this pull requestMay 4, 2025
…xabbuh)This PR was merged into the 7.3 branch.Discussion----------[DebugBundle] require the 7.3+ of the Config component| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        || License       | MITrelated to#59762, we could bump the conflict rule instead, but I don't see a use case where using the bundle without the Config component actually makes senseCommits-------28d1a83 require the 7.3+ of the Config component
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@GromNaNGromNaNGromNaN left review comments

@stofstofstof left review comments

@OskarStarkOskarStarkOskarStark left review comments

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

7.3

Development

Successfully merging this pull request may close these issues.

7 participants

@alexandre-daubois@GromNaN@stof@fabpot@nicolas-grekas@OskarStark@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp