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

Add must usage in coding standards #20468

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

Conversation

alexandre-daubois
Copy link
Contributor

A recurring topic in code PRs is the use of “must” in exception messages. As @chalasr points out in symfony/symfony#59177 (comment), this does not appear to be a decision written into the code standards of the project.

I suggest we add a rule to the standard coding, indicating that “must” should never be used in messages intended for the user. Indeed, it is a feedback I had a few times on some of my PRs. I know that the docs repository tries to ban the use of "must", except in very rare occasions.

What do you think? Discussion is open 🙂

@alexandre-daubois
Copy link
Contributor Author

Status: Waiting Team Decision

@chalasr
Copy link
Member

chalasr commented Dec 15, 2024

Although I understand the intent behind this proposal, I'm against adding such a statement as I don't think it is all or nothing.
Docs are mostly about educating, providing guidance about how the framework should be used. On the other hand, code is specification and implementation that state what is strictly expected to make things work.

While it makes sense to use softer wording in guides, I don't think it is wrong to use stronger - more imperative - wording in specifications and the errors/notices that are triggered by implementations and callers.

To me, it doesn't hurt and has the benefit to make things clear.
Also correct me if I'm wrong but looking at other frameworks and languages code, they all seem to use "must" as we do.

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