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

Include the acronym casing RFC in coding-standards-and-naming.rst #7

Merged
merged 1 commit into from
May 28, 2024

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented May 8, 2024

Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

I don't know if the policy should be an evolving document, Derick will know that.
But the contents itself look good.

Copy link
Contributor

@derickr derickr left a comment

Choose a reason for hiding this comment

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

The idea behind these documents is that they update the actual text — not adding a new section that has changes to already existing text.

At the top, you can do something like:

https://github.com/php/policies/blob/main/release-process.rst?plain=1#L8

@TimWolla
Copy link
Member Author

The idea behind these documents is that they update the actual text — not adding a new section that has changes to already existing text.

My understanding is that the coding-standards-and-naming.rst has not yet been cleaned up following the “Policy Repository” RFC:

Once (and if) this RFC is accepted, a first new step would be to rephrase the text so that it reads like a policy document, instead of an RFC. The wording is currently exactly as in the used RFCs, without modification.

… and thus still is the concatenation of the relevant RFCs instead of a properly written policy document.

I feel that editing around in the existing non-cleaned-up state would make things worse rather than better, because the result is neither a coherent text, nor does it match the RFC texts that are authoritative as-of now.

@TimWolla
Copy link
Member Author

In fact as per my understanding of the “Policy Repository” RFC, the cleaning up requires another RFC to confirm that no unintended changes have been introduced:

Both of these will be implemented through a separate PR to the policy repository. After comment integration, an RFC will be created to confirm these changes.

The previous PR for the release cycle update is already merged and thus to my understanding violates that decision.

@derickr
Copy link
Contributor

derickr commented May 14, 2024

Hmm, you're right. I had missed that from my own RFC :-). The question is... does it actually make sense to do?

But I would also say, that it probably have been wise to cover that in your "acronym casing policy" RFC so that we don't need to do it twice!

@derickr
Copy link
Contributor

derickr commented May 20, 2024

OK, please merge this then. I will then start working on fixing up all the language and put it up for an RFC.

@TimWolla
Copy link
Member Author

Sorry, didn't get around to this over the weekend and was busy last week.

But I would also say, that it probably have been wise to cover that in your "acronym casing policy" RFC so that we don't need to do it twice!

I've written the RFC in a way such that it makes a well-defined change to the CODING_STANDARDS.md. That one is the more complete file, compared to the stuff in the policies repository and thus is probably the best source for the rewrite.

OK, please merge this then.

I don't have the commit bit for this repository, someone else will need to merge.

I will then start working on fixing up all the language and put it up for an RFC.

Thank you.

@derickr derickr merged commit 2f760d3 into php:main May 28, 2024
@TimWolla TimWolla deleted the acronym-casing-policy branch May 28, 2024 18:04
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.

None yet

3 participants