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

fix: path references in YAML config snippets #2123

Open
wants to merge 1 commit into
base: 2.6
Choose a base branch
from

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Jan 9, 2025

AFAIK, the documentation is built around the paths from api-platform/api-platform.

  • Added missing api/ directory references where missing;
  • Replaced app/ with api/.

Copy link
Member

@alanpoulain alanpoulain left a comment

Choose a reason for hiding this comment

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

Seems OK to me, @dunglas do you agree?

@soyuka
Copy link
Member

soyuka commented Jan 13, 2025

core is the symfony-only version in a sense, I'd say that the path is fine without the api prefix.

@phansys phansys force-pushed the config_paths branch 3 times, most recently from c90713b to 01c30ed Compare January 13, 2025 16:23
@phansys
Copy link
Contributor Author

phansys commented Jan 13, 2025

core is the symfony-only version in a sense, I'd say that the path is fine without the api prefix.

I agree, but I also think we should make explicit clarifications in places where the distribution version is referenced, as there are some examples where the removal of the api/ prefix may lead to misleading results.

I mean cases like these:

IIUC, the only places where the api/ prefix is right is where the documentation is covering the setup for the api-platform/api-platform distribution. Other examples for use with Symfony or Laravel SHOULD NOT use the api/ prefix.

Do you agree? I'll be waiting for your feedback in order to confirm what updates to make.

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

Successfully merging this pull request may close these issues.

3 participants