[RFC]: Records #1
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughA new feature in PHP introduces Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (21)
drafts/records.md (11)
6-6: Consider simplifying the phrase "Under Discussion".The phrase "Under Discussion" might be wordy. Consider using "In Discussion" or "Discussing".
- Status: Draft (or Under Discussion or Accepted or Declined) + Status: Draft (or In Discussion or Accepted or Declined)Tools
LanguageTool
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
7-7: Avoid using bare URLs.The URL should be formatted as a proper hyperlink to comply with Markdownlint rules.
- First Published at: <http://wiki.php.net/rfc/records> + First Published at: [http://wiki.php.net/rfc/records](http://wiki.php.net/rfc/records)
21-21: Consider simplifying the phrase "new record".The phrase "new record" might be wordy. Consider using "record".
- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects.Tools
LanguageTool
[style] ~21-~21: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
43-43: Use "different from" instead of "different than".The phrase "different than" is often considered colloquial style. Use "different from" instead.
- Regular properties may also be defined, but they are immutable by default and are no different than `const`. + Regular properties may also be defined, but they are immutable by default and are no different from `const`.Tools
LanguageTool
[style] ~43-~43: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...mutable by default and are no different thanconst. Static properties and methods...(DIFFERENT_THAN)
47-47: Avoid redundancy in the phrase "always and forever".The phrase "always and forever" is redundant. Consider using "always" or "forever".
- public int $height = 1; // this will always and forever be "1", it is immutable. + public int $height = 1; // this will always be "1", it is immutable.
71-71: Consider replacing the phrasal verb "leave out".To strengthen your wording, consider replacing "leave out" with "omit".
- A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.
82-82: Consider simplifying the phrase "new record".The phrase "new record" might be wordy. Consider using "record".
- and it returns a new record object with the given values. + and it returns a record object with the given values.
109-109: Consider simplifying the phrase "exact same".The phrase "exact same" might be wordy. Consider using "same".
- Calling `clone` on a `record` results in the exact same record object being returned. + Calling `clone` on a `record` results in the same record object being returned.Tools
LanguageTool
[style] ~109-~109: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
167-167: Ensure the sentence has a subject.To form a complete sentence, be sure to include a subject.
- `ReflectionClass` can be used to inspect records, their properties, and methods. + The `ReflectionClass` can be used to inspect records, their properties, and methods.Tools
LanguageTool
[style] ~167-~167: To form a complete sentence, be sure to include a subject.
Context: ...lectionClass supportReflectionClasscan be used to inspect records, their prope...(MISSING_IT_THERE)
234-241: Correct the spelling of "auto loading".The word "auto loading" should be spelled as one word "autoloading".
- ### Auto loading + ### Autoloading - and PHP has no function autoloader, auto loading will not be supported in this implementation. + and PHP has no function autoloader, autoloading will not be supported in this implementation. - If function auto loading were to be implemented in the future, an autoloader could locate the `record` and autoload it. + If function autoloading were to be implemented in the future, an autoloader could locate the `record` and autoload it. - someone to put forward a function auto loading RFC if auto loading is desired for records. + someone to put forward a function autoloading RFC if autoloading is desired for records.Tools
LanguageTool
[misspelling] ~234-~234: This word is normally spelled as one.
Context: ...d afunctionwith the same name. ### Auto loading As invoking a record value by its name...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~237-~237: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~238-~238: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~241-~241: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~241-~241: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ## Backward In...(EN_COMPOUNDS_AUTO_LOADING)
275-275: Correct the spelling of "Todo".It appears that a hyphen is missing in the noun "To-do".
- Todo + To-doTools
LanguageTool
[grammar] ~275-~275: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...php.ini Defaults None ## Open Issues Todo ## Unaffected PHP Functionality None....(TO_DO_HYPHEN)
published/records.txt (10)
6-6: Consider simplifying the phrase "Under Discussion".The phrase "Under Discussion" might be wordy. Consider using "In Discussion" or "Discussing".
- * Status: Draft (or Under Discussion or Accepted or Declined) + * Status: Draft (or In Discussion or Accepted or Declined)Tools
LanguageTool
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected]> * Status: Draft (or Under Discussion or Accepted or Declined) * First Publ...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
15-15: Consider simplifying the phrase "new record".The phrase "new record" might be wordy. Consider using "record".
- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects.Tools
LanguageTool
[style] ~15-~15: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
25-25: Use "different from" instead of "different than".The phrase "different than" is often considered colloquial style. Use "different from" instead.
- Regular properties may also be defined, but they are immutable by default and are no different than ''%%const%%''. + Regular properties may also be defined, but they are immutable by default and are no different from ''%%const%%''.Tools
LanguageTool
[style] ~25-~25: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...mutable by default and are no different than ''%%const%%''. Static properties and m...(DIFFERENT_THAN)
47-47: Avoid redundancy in the phrase "always and forever".The phrase "always and forever" is redundant. Consider using "always" or "forever".
- public int $height = 1; // this will always and forever be "1", it is immutable. + public int $height = 1; // this will always be "1", it is immutable.Tools
LanguageTool
[style] ~47-~47: This phrase is redundant. Consider writing “always” or “forever”.
Context: ... public int $height = 1; // this will always and forever be "1", it is immutable. public int ...(ALWAYS_AND_FOREVER)
71-71: Consider replacing the phrasal verb "leave out".To strengthen your wording, consider replacing "leave out" with "omit".
- A ''%%record%%'' can also be defined with optional parameters that are set if left out during instantiation. + A ''%%record%%'' can also be defined with optional parameters that are set if omitted during instantiation.Tools
LanguageTool
[style] ~71-~71: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation.recor...(OMIT_EXCLUDE)
82-82: Consider simplifying the phrase "new record".The phrase "new record" might be wordy. Consider using "record".
- and it returns a new record object with the given values. + and it returns a record object with the given values.Tools
LanguageTool
[style] ~82-~82: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. <code ph...(EN_WORDINESS_PREMIUM_NEW_RECORD)
109-109: Consider simplifying the phrase "exact same".The phrase "exact same" might be wordy. Consider using "same".
- Calling ''%%clone%%'' on a ''%%record%%'' results in the exact same record object being returned. + Calling ''%%clone%%'' on a ''%%record%%'' results in the same record object being returned.Tools
LanguageTool
[style] ~109-~109: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...%%'' on a ''%%record%%'' results in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
125-125: Ensure the sentence has a subject.To form a complete sentence, be sure to include a subject.
- ''%%ReflectionClass%%'' can be used to inspect records, their properties, and methods. + The ''%%ReflectionClass%%'' can be used to inspect records, their properties, and methods.
184-186: Correct the spelling of "auto loading".The word "auto loading" should be spelled as one word "autoloading".
- ==== Auto loading ==== + ==== Autoloading ==== - and PHP has no function autoloader, auto loading will not be supported in this implementation. + and PHP has no function autoloader, autoloading will not be supported in this implementation. - If function auto loading were to be implemented in the future, an autoloader could locate the ''%%record%%'' and autoload it. + If function autoloading were to be implemented in the future, an autoloader could locate the ''%%record%%'' and autoload it. - someone to put forward a function auto loading RFC if auto loading is desired for records. + someone to put forward a function autoloading RFC if autoloading is desired for records.Tools
LanguageTool
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ===== Backward...(EN_COMPOUNDS_AUTO_LOADING)
220-220: Correct the spelling of "Todo".It appears that a hyphen is missing in the noun "To-do".
- Todo + To-do
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Commits
Files that changed from the base of the PR and between 421b3205bb353d0fa3b6b7df574f537fef2a14d0 and b7a8603cfac1b8d0ea80fc7dfd4804bbf7255231.
Files selected for processing (2)
- drafts/records.md (1 hunks)
- published/records.txt (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern drafts/*.md: These are PHP RFC's to change the PHP language. Be constructive but critical in how it may change the language
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~21-~21: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~43-~43: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...mutable by default and are no different than const. Static properties and methods...
(DIFFERENT_THAN)
[style] ~93-~93: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...
(OMIT_EXCLUDE)
[style] ~109-~109: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~141-~141: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ng clone on a record results in the exact same record object being returned. As it is ...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
[style] ~167-~167: To form a complete sentence, be sure to include a subject.
Context: ...lectionClass support ReflectionClass can be used to inspect records, their prope...
(MISSING_IT_THERE)
[misspelling] ~234-~234: This word is normally spelled as one.
Context: ...d a function with the same name. ### Auto loading As invoking a record value by its name...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~237-~237: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~238-~238: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~241-~241: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~241-~241: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ## Backward In...
(EN_COMPOUNDS_AUTO_LOADING)
[grammar] ~275-~275: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...php.ini Defaults None ## Open Issues Todo ## Unaffected PHP Functionality None....
(TO_DO_HYPHEN)
published/records.txt
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected]> * Status: Draft (or Under Discussion or Accepted or Declined) * First Publ...
(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~15-~15: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~25-~25: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...mutable by default and are no different than ''%%const%%''. Static properties and m...
(DIFFERENT_THAN)
[style] ~47-~47: This phrase is redundant. Consider writing “always” or “forever”.
Context: ... public int $height = 1; // this will always and forever be "1", it is immutable. public int ...
(ALWAYS_AND_FOREVER)
[style] ~71-~71: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. recor...
(OMIT_EXCLUDE)
[style] ~82-~82: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. <code ph...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~109-~109: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...%%'' on a ''%%record%%'' results in the exact same record object being returned. As it is ...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...
(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~186-~186: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ===== Backward...
(EN_COMPOUNDS_AUTO_LOADING)
[grammar] ~219-~219: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...ts ==== None ===== Open Issues ===== Todo ===== Unaffected PHP Functionality ===...
(TO_DO_HYPHEN)
Markdownlint
drafts/records.md
28-28: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
5-5: null
Bare URL used
(MD034, no-bare-urls)
221-221: Expected: fenced; Actual: indented
Code block style
(MD046, code-block-style)
Additional comments not posted (6)
drafts/records.md (3)
9-17: LGTM!
The introduction is clear and well-written.
244-246: LGTM!
The section on backward incompatible changes is clear and well-written.
282-284: LGTM!
The section on future scope is clear and well-written.
published/records.txt (3)
9-12: LGTM!
The introduction is clear and well-written.
188-190: LGTM!
The section on backward incompatible changes is clear and well-written.
226-228: LGTM!
The section on future scope is clear and well-written.
withinboredom
force-pushed
the
add/records
branch
from
b7a8603 to
c69bff3
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (5)
drafts/records.md (5)
43-44: Simplify the phrase.The phrase "Like arrays, strings, and other values" can be simplified to "Like arrays and strings".
- Like arrays, strings, and other values, `record` objects are strongly equal to each other if they contain the same + Like arrays and strings, `record` objects are strongly equal to each other if they contain the same
65-65: Simplify the phrase.The phrase "new record" can be simplified to "record".
- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. These objects will + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects. These objects willTools
LanguageTool
[style] ~65-~65: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
124-124: Strengthen the wording.Replace "left out" with "omitted" for stronger wording.
- A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.Tools
LanguageTool
[style] ~124-~124: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
138-138: Simplify the phrase.The phrase "new record" can be simplified to "record".
- No other property names can be used, and it returns a new record object with the given values. + No other property names can be used, and it returns a record object with the given values.Tools
LanguageTool
[style] ~138-~138: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
205-205: Simplify the phrase.The phrase "exact same" can be simplified to "same".
- Calling `clone` on a `record` results in the exact same record object being returned. + Calling `clone` on a `record` results in the same record object being returned.Tools
LanguageTool
[style] ~205-~205: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ngcloneon arecordresults in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (2)
- Makefile (1 hunks)
- drafts/records.md (1 hunks)
Files skipped from review due to trivial changes (1)
- Makefile
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Be constructive but critical in how it may change the language
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[grammar] ~16-~16: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...bjects that represent a value. They are used to store values with a different meaning than th...(ADMIT_ENJOY_VB)
[style] ~65-~65: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[typographical] ~80-~80: Usually, there’s no comma before “if”.
Context: ...uments to perform further initialization, if required. If it does not have a constru...(IF_NO_COMMA)
[style] ~124-~124: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
[style] ~138-~138: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~205-~205: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ngcloneon arecordresults in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
[style] ~229-~229: To form a complete sentence, be sure to include a subject.
Context: ...lectionClass supportReflectionClasscan be used to inspect records, their prope...(MISSING_IT_THERE)
[misspelling] ~289-~289: This word is normally spelled as one.
Context: ...d afunctionwith the same name. ### Auto loading As invoking a record value by its name...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~292-~292: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~293-~293: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~294-~294: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~294-~294: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ## Backward In...(EN_COMPOUNDS_AUTO_LOADING)
[grammar] ~327-~327: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...php.ini Defaults None ## Open Issues Todo ## Unaffected PHP Functionality None....(TO_DO_HYPHEN)
Markdownlint
drafts/records.md
71-71: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: null
Bare URL used(MD034, no-bare-urls)
277-277: Expected: fenced; Actual: indented
Code block style(MD046, code-block-style)
Additional comments not posted (2)
drafts/records.md (2)
14-21: LGTM!The explanation and example for value objects are clear and effective.
Tools
LanguageTool
[grammar] ~16-~16: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...bjects that represent a value. They are used to store values with a different meaning than th...(ADMIT_ENJOY_VB)
298-298: LGTM!The section clearly states that there are no backward incompatible changes.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Rob Landers <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (2)
drafts/records.md (2)
65-65: Consider reducing wordiness.The phrase "new record" might be wordy. Consider a shorter alternative.
- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects.- No other property names can be used, and it returns a new record object with the given values. + No other property names can be used, and it returns a record object with the given values.Also applies to: 138-138
Tools
LanguageTool
[style] ~65-~65: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
124-124: Consider strengthening wording.To strengthen your wording, consider replacing the phrasal verb “leave out”.
- A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.Tools
LanguageTool
[style] ~124-~124: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (1)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Be constructive but critical in how it may change the language
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~65-~65: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[typographical] ~80-~80: Usually, there’s no comma before “if”.
Context: ...uments to perform further initialization, if required. If it does not have a constru...(IF_NO_COMMA)
[style] ~124-~124: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
[style] ~138-~138: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~205-~205: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ngcloneon arecordresults in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
[style] ~229-~229: To form a complete sentence, be sure to include a subject.
Context: ...lectionClass supportReflectionClasscan be used to inspect records, their prope...(MISSING_IT_THERE)
[misspelling] ~289-~289: This word is normally spelled as one.
Context: ...d afunctionwith the same name. ### Auto loading As invoking a record value by its name...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~292-~292: This word is normally spelled as one.
Context: ...on, and PHP has no function autoloader, auto loading will not be supported in this implement...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~293-~293: This word is normally spelled as one.
Context: ...ted in this implementation. If function auto loading were to be implemented in the future, a...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~294-~294: This word is normally spelled as one.
Context: ...rages someone to put forward a function auto loading RFC if auto loading is desired for reco...(EN_COMPOUNDS_AUTO_LOADING)
[misspelling] ~294-~294: This word is normally spelled as one.
Context: ... forward a function auto loading RFC if auto loading is desired for records. ## Backward In...(EN_COMPOUNDS_AUTO_LOADING)
[grammar] ~327-~327: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...php.ini Defaults None ## Open Issues Todo ## Unaffected PHP Functionality None....(TO_DO_HYPHEN)
Markdownlint
drafts/records.md
71-71: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
5-5: null
Bare URL used(MD034, no-bare-urls)
277-277: Expected: fenced; Actual: indented
Code block style(MD046, code-block-style)
Additional comments not posted (6)
drafts/records.md (6)
80-80: Remove unnecessary comma.The comma before "if required" is unnecessary.
- A `record` may contain a constructor with zero arguments to perform further initialization, if required. + A `record` may contain a constructor with zero arguments to perform further initialization if required.Tools
LanguageTool
[typographical] ~80-~80: Usually, there’s no comma before “if”.
Context: ...uments to perform further initialization, if required. If it does not have a constru...(IF_NO_COMMA)
229-229: Fix missing subject.Add "It" to complete the sentence.
- `ReflectionClass` can be used to inspect records, their properties, and methods. + It can be used to inspect records, their properties, and methods.Tools
LanguageTool
[style] ~229-~229: To form a complete sentence, be sure to include a subject.
Context: ...lectionClass supportReflectionClasscan be used to inspect records, their prope...(MISSING_IT_THERE)
289-289: Fix misspelling.The term "auto loading" should be spelled as one word "autoloading".
- ### Auto loading + ### Autoloading - and PHP has no function autoloader, auto loading will not be supported in this implementation. + and PHP has no function autoloader, autoloading will not be supported in this implementation. - If function auto loading were to be implemented in the future, an autoloader could locate the `record` and autoload it. + If function autoloading were to be implemented in the future, an autoloader could locate the `record` and autoload it. - The author of this RFC strongly encourages someone to put forward a function auto loading RFC if auto loading is desired for records. + The author of this RFC strongly encourages someone to put forward a function autoloading RFC if autoloading is desired for records.Also applies to: 292-294
Tools
LanguageTool
[misspelling] ~289-~289: This word is normally spelled as one.
Context: ...d afunctionwith the same name. ### Auto loading As invoking a record value by its name...(EN_COMPOUNDS_AUTO_LOADING)
327-327: Fix misspelling.The term "Todo" should be spelled as "To-do".
- Todo + To-doTools
LanguageTool
[grammar] ~327-~327: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: ...php.ini Defaults None ## Open Issues Todo ## Unaffected PHP Functionality None....(TO_DO_HYPHEN)
334-334: Add content to the Future Scope section.The section is currently empty. Consider adding potential future enhancements or considerations.
Do you want me to provide suggestions for potential future enhancements or considerations for this section?
16-16: Fix grammatical issue.The phrase "used to store values" should be "used for storing values" to be grammatically correct.
- Value objects are immutable objects that represent a value. They are used to store values with a different meaning than + Value objects are immutable objects that represent a value. They are used for storing values with a different meaning thanLikely invalid or redundant comment.
withinboredom
force-pushed
the
add/records
branch
from
f17ff26 to
be7ae31
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (2)
- .coderabbit.yml (1 hunks)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Be constructive but critical in how it may change the language
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~68-~68: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~127-~127: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
[style] ~141-~141: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~208-~208: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ngcloneon arecordresults in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
Markdownlint
drafts/records.md
5-5: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (9)
.coderabbit.yml (1)
6-6: Consider the implications of changing the review profile toassertive.This change will make the review feedback more direct and critical. Ensure this aligns with the team's expectations and communication style.
drafts/records.md (8)
16-16: Fix grammatical issue.The phrase "used for storing values" is grammatically correct and should be retained.
329-329: Fix misspelling.The term "autoloading" should be spelled as one word.
367-367: Fix misspelling.The term "Todo" should be spelled as "To-do".
374-374: Add content to the Future Scope section.The section is currently empty. Consider adding potential future enhancements or considerations.
383-383: Add patches and tests.The section is currently empty. Once patches and tests are available, this section should be populated with relevant information.
387-393: Add implementation details.The section is currently empty. Once the project is implemented, this section should contain details about the version(s) it was merged into, links to the git commit(s), PHP manual entry, and language specification section (if any).
396-396: Add references.The section is currently empty. Consider adding links to external references, discussions, or RFCs.
400-400: Add rejected features.The section is currently empty. Consider adding features that were discussed but not included in the final proposal.
There was a problem hiding this comment.
Actionable comments posted: 12
Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (1)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Be constructive but critical in how it may change the language
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~68-~68: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~127-~127: To strengthen your wording, consider replacing the phrasal verb “leave out”.
Context: ...ith optional parameters that are set if left out during instantiation. ``` php record R...(OMIT_EXCLUDE)
[style] ~141-~141: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...rty names can be used, and it returns a new record object with the given values. ``` php ...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[style] ~208-~208: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ngcloneon arecordresults in the exact same record object being returned. As it is ...(EN_WORDINESS_PREMIUM_EXACT_SAME)
Markdownlint
drafts/records.md
5-5: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (3)
drafts/records.md (3)
41-61: LGTM!The explanation of the solution using
recordobjects is clear and concise.
265-265: Fix missing subject.Add "It" to complete the sentence.
- `ReflectionClass` can be used to inspect records, their properties, and methods. + It can be used to inspect records, their properties, and methods.Likely invalid or redundant comment.
16-16: Fix grammatical issue.The phrase "used to store values" should be "used for storing values" to be grammatically correct.
- Value objects are immutable objects that represent a value. They are used to store values with a different meaning than + Value objects are immutable objects that represent a value. They are used for storing values with a different meaning thanLikely invalid or redundant comment.
|
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (2)
- .coderabbit.yml (2 hunks)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Do not just consider the grammar of the text, but consider how it might change the language. For example, if a new feature is being added, consider how it might be used, and propose better ideas if you have them.
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[style] ~68-~68: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
Markdownlint
drafts/records.md
5-5: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (13)
.coderabbit.yml (2)
6-6: Review tone changed to assertive.The review profile has been updated to
assertive, indicating a shift towards more direct and critical feedback.
21-21: Expanded path instructions for PHP RFCs.The instructions now emphasize a broader consideration of the implications of proposed changes to the PHP language.
drafts/records.md (11)
11-11: Simplify wording.Consider removing "new" to improve readability.
[style]- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects.
16-16: Fix grammatical issue.The phrase "used to store values" should be "used for storing values" to be grammatically correct.
[grammar]- Value objects are immutable objects that represent a value. They are used to store values with a different meaning than + Value objects are immutable objects that represent a value. They are used for storing values with a different meaning than
41-65: LGTM!The solution is well-explained and the example is clear.
68-68: Simplify wording.Consider removing "new" to improve readability.
[style]- This RFC proposes the introduction of a new record keyword in PHP to define immutable data objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable data objects.Tools
LanguageTool
[style] ~68-~68: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...This RFC proposes the introduction of a new record keyword in PHP to define immutable data...(EN_WORDINESS_PREMIUM_NEW_RECORD)
74-92: LGTM!The definition is clear and covers various aspects of
recordobjects.
120-124: LGTM!The usage examples are clear and helpful.
127-127: Strengthen wording.Consider replacing "left out" with "omitted" for clarity.
[style]- A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.
141-141: Simplify wording.Consider removing "new" to improve readability.
[style]- No other property names can be used, and it returns a new record object with the given values. + No other property names can be used, and it returns a record object with the given values.
169-190: LGTM!The explanation is clear and the examples are helpful.
191-204: LGTM!The explanation is clear and covers various aspects of performance.
208-208: Simplify wording.Consider removing "exact" to improve readability.
[style]- Calling `clone` on a `record` results in the exact same record object being returned. + Calling `clone` on a `record` results in the same record object being returned.
|
@coderabbitai full review |
Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 25
Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (1)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Do not just consider the grammar of the text, but consider how it might change the language. For example, if a new feature is being added, consider how it might be used, and propose better ideas if you have them.
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[grammar] ~17-~17: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...objects that represent a value. They’re used to store values with a different semantic meanin...(ADMIT_ENJOY_VB)
[grammar] ~346-~346: After ‘it’, use the third-person verb form “is”.
Context: ...it can work in the above model, then it be possible. ### Performance consideratio...(IT_VBZ)
[uncategorized] ~432-~432: Loose punctuation mark.
Context: ...inalizeRecord(object $instance): Record`: Finalizes a record under construction, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~433-~433: Loose punctuation mark.
Context: ...onRecord::isRecord(mixed $object): bool: Returnstrue` if the object is a recor...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...InlineConstructor(): ReflectionFunction`: Returns the inline constructor of the r...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~435-~435: Loose punctuation mark.
Context: ...ditionalConstructor(): ReflectionMethod`: Returns the traditional constructor of ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~436-~436: Loose punctuation mark.
Context: ...::makeMutable(Record $instance): object`: Returns a new record instance with the ...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~436-~436: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...e(Record $instance): object`: Returns a new record instance with the properties mutable. -...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[uncategorized] ~437-~437: Loose punctuation mark.
Context: ...cord::isMutable(Record $instance): bool: Returnstrue` if the record is mutable...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
drafts/records.md
70-70: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
223-223: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
5-5: null
Bare URL used(MD034, no-bare-urls)
178-178: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
183-183: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Additional comments not posted (6)
drafts/records.md (6)
432-437: Fix loose punctuation marks.There are loose punctuation marks in the method signatures.
- `ReflectionRecord::finalizeRecord(object $instance): Record`: Finalizes a record under construction, making it immutable. - `ReflectionRecord::isRecord(mixed $object): bool`: Returns `true` if the object is a record, and `false` otherwise. - `ReflectionRecord::getInlineConstructor(): ReflectionFunction`: Returns the inline constructor of the record as `ReflectionFunction`. <details> <summary>Tools</summary> <details> <summary>LanguageTool</summary><blockquote> [uncategorized] ~432-~432: Loose punctuation mark. Context: ...inalizeRecord(object $instance): Record`: Finalizes a record under construction, ... (UNLIKELY_OPENING_PUNCTUATION) --- [uncategorized] ~433-~433: Loose punctuation mark. Context: ...onRecord::isRecord(mixed $object): bool`: Returns `true` if the object is a recor... (UNLIKELY_OPENING_PUNCTUATION) --- [uncategorized] ~434-~434: Loose punctuation mark. Context: ...InlineConstructor(): ReflectionFunction`: Returns the inline constructor of the r... (UNLIKELY_OPENING_PUNCTUATION) --- [uncategorized] ~435-~435: Loose punctuation mark. Context: ...ditionalConstructor(): ReflectionMethod`: Returns the traditional constructor of ... (UNLIKELY_OPENING_PUNCTUATION) --- [uncategorized] ~436-~436: Loose punctuation mark. Context: ...::makeMutable(Record $instance): object`: Returns a new record instance with the ... (UNLIKELY_OPENING_PUNCTUATION) --- [style] ~436-~436: ‘new record’ might be wordy. Consider a shorter alternative. Context: ...e(Record $instance): object`: Returns a new record instance with the properties mutable. -... (EN_WORDINESS_PREMIUM_NEW_RECORD) --- [uncategorized] ~437-~437: Loose punctuation mark. Context: ...cord::isMutable(Record $instance): bool`: Returns `true` if the record is mutable... (UNLIKELY_OPENING_PUNCTUATION) </blockquote></details> </details> --- `158-158`: **Strengthen wording.** Consider replacing "left out" with "omitted" for clarity. ```diff - A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.Likely invalid or redundant comment.
17-17: Fix grammatical issue.The phrase "used to store values" should be "used for storing values" to be grammatically correct.
- They’re used to store values with a different semantic meaning than their technical value, adding additional context. + They’re used for storing values with a different semantic meaning than their technical value, adding additional context.Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~17-~17: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...objects that represent a value. They’re used to store values with a different semantic meanin...(ADMIT_ENJOY_VB)
180-181: Clarify the named arguments restriction.Consider rephrasing for clarity.
- The `with` method accepts only named arguments defined in the inline constructor. Properties not defined in the inline constructor can’t be updated by this method. + The `with` method accepts only named arguments defined in the inline constructor. Properties not defined in the inline constructor cannot be updated by this method.Likely invalid or redundant comment.
70-70: Simplify wording.Consider removing "new" to improve readability.
- This RFC proposes the introduction of a `record` keyword in PHP to define immutable value objects. + This RFC proposes the introduction of a `record` keyword in PHP to define immutable value objects.Likely invalid or redundant comment.
Tools
Markdownlint
70-70: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
96-97: Fix missing comma.It appears that a comma is missing.
- At any other time the property is immutable. + At any other time, the property is immutable.Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 15
Review details
Configuration used: .coderabbit.yml
Review profile: ASSERTIVE
Files ignored due to path filters (1)
published/records.ptxtis excluded by!**/*.ptxt
Files selected for processing (1)
- drafts/records.md (1 hunks)
Additional context used
Path-based instructions (1)
drafts/records.md (1)
Pattern
drafts/*.md: These are PHP RFC's to change the PHP language. Do not just consider the grammar of the text, but consider how it might change the language. For example, if a new feature is being added, consider how it might be used, and propose better ideas if you have them.
LanguageTool
drafts/records.md
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
[grammar] ~17-~17: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...objects that represent a value. They’re used to store values with a different semantic meanin...(ADMIT_ENJOY_VB)
[uncategorized] ~90-~90: Possible missing preposition found.
Context: ...aces{}. A record may not extend another record or class. A record may cont...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~270-~270: This verb does not appear to agree with the subject. Consider using a different form.
Context: ... } } ``` ### Mental models and how it works From the perspective of a developer, d...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[grammar] ~347-~347: After ‘it’, use the third-person verb form “is”.
Context: ...it can work in the above model, then it be possible. ### Performance consideratio...(IT_VBZ)
[uncategorized] ~433-~433: Loose punctuation mark.
Context: ...inalizeRecord(object $instance): Record`: Finalizes a record under construction, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...onRecord::isRecord(mixed $object): bool: Returnstrue` if the object is a recor...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~435-~435: Loose punctuation mark.
Context: ...InlineConstructor(): ReflectionFunction`: Returns the inline constructor of the r...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~436-~436: Loose punctuation mark.
Context: ...ditionalConstructor(): ReflectionMethod`: Returns the traditional constructor of ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~437-~437: Loose punctuation mark.
Context: ...::makeMutable(Record $instance): object`: Returns a new record instance with the ...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~437-~437: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...e(Record $instance): object`: Returns a new record instance with the properties mutable. -...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[uncategorized] ~438-~438: Loose punctuation mark.
Context: ...cord::isMutable(Record $instance): bool: Returnstrue` if the record is mutable...(UNLIKELY_OPENING_PUNCTUATION)
Markdownlint
drafts/records.md
223-223: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
5-5: null
Bare URL used(MD034, no-bare-urls)
178-178: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
183-183: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Additional comments not posted (19)
drafts/records.md (19)
6-6: Simplify the status description.Consider using a shorter alternative for "Under Discussion" to improve readability.
[style]- Status: Draft (or Under Discussion or Accepted or Declined) + Status: Draft (or Discussing or Accepted or Declined)Tools
LanguageTool
[style] ~6-~6: ‘Under Discussion’ might be wordy. Consider a shorter alternative.
Context: [email protected] - Status: Draft (or Under Discussion or Accepted or Declined) - First Publis...(EN_WORDINESS_PREMIUM_UNDER_DISCUSSION)
17-17: Fix grammatical issue.The phrase "used to store values" should be "used for storing values" to be grammatically correct.
[grammar]- They’re used to store values with a different semantic meaning than their technical value, + They’re used for storing values with a different semantic meaning than their technical value,Tools
LanguageTool
[grammar] ~17-~17: In the grammatical structure ‘pronoun + be/get + used + to + verb’, the verb ‘used’ is used with the gerund.
Context: ...objects that represent a value. They’re used to store values with a different semantic meanin...(ADMIT_ENJOY_VB)
23-23: Fix misspelling.The term "non-sensical" should be spelled as "nonsensical".
[misspelling]- and the ID is accidentally set to a non-sensical value: + and the ID is accidentally set to a nonsensical value:
35-35: Fix typographical error.The phrase "accidental passes a nonsensical value for uid" should be corrected.
[typographical]- accidental passes a nonsensical value for uid + accidentally passes a nonsensical value for uid
70-70: Simplify wording.Consider removing "new" to improve readability.
[style]- This RFC proposes the introduction of a new record keyword in PHP to define immutable value objects. + This RFC proposes the introduction of a record keyword in PHP to define immutable value objects.
90-90: Fix missing preposition.A preposition is missing in the sentence.
[grammar]- A **record** may not extend another record or class. + A **record** may not extend from another record or class.Tools
LanguageTool
[uncategorized] ~90-~90: Possible missing preposition found.
Context: ...aces{}. A record may not extend another record or class. A record may cont...(AI_EN_LECTOR_MISSING_PREPOSITION)
96-97: Fix missing comma.It appears that a comma is missing.
[typographical]- At any other time the property is immutable. + At any other time, the property is immutable.
158-158: Strengthen wording.Consider replacing "left out" with "omitted" for clarity.
[style]- A `record` can also be defined with optional parameters that are set if left out during instantiation. + A `record` can also be defined with optional parameters that are set if omitted during instantiation.
347-347: Fix grammatical issue.After ‘it’, use the third-person verb form “is”.
[grammar]- In other words, if it can work in the above model, then it be possible. + In other words, if it can work in the above model, then it is possible.Tools
LanguageTool
[grammar] ~347-~347: After ‘it’, use the third-person verb form “is”.
Context: ...it can work in the above model, then it be possible. ### Performance consideratio...(IT_VBZ)
359-359: Fix typographical error.The phrase "it is pointing the the same memory as $point1" should be corrected.
[typographical]- it is pointing the the same memory as $point1 + it is pointing to the same memory as $point1
366-366: Simplify wording.Consider removing "exact" to improve readability.
[style]- Calling `clone` on a `record` results in the exact same record object being returned. + Calling `clone` on a `record` results in the same record object being returned.
499-499: Fix missing comma.It appears that a comma is missing.
[typographical]- When passed an instance of a record the `var_dump()` function will output the same + When passed an instance of a record, the `var_dump()` function will output the same
433-438: Fix loose punctuation marks.There are loose punctuation marks in these lines.
[typographical]- `ReflectionRecord::finalizeRecord(object $instance): Record`: Finalizes a record under construction, making it immutable. - `ReflectionRecord::isRecord(mixed $object): bool`: Returns `true` if the object is a record, and `false` otherwise. - `ReflectionRecord::getInlineConstructor(): ReflectionFunction`: Returns the inline constructor of the record as `ReflectionFunction`. - `ReflectionRecord::getTraditionalConstructor(): ReflectionMethod`: Returns the traditional constructor of the record as `ReflectionMethod`. - `ReflectionRecord::makeMutable(Record $instance): object`: Returns a new record instance with the properties mutable. - `ReflectionRecord::isMutable(Record $instance): bool`: Returns `true` if the record is mutable, and `false` otherwise. + `ReflectionRecord::finalizeRecord(object $instance): Record` - Finalizes a record under construction, making it immutable. + `ReflectionRecord::isRecord(mixed $object): bool` - Returns `true` if the object is a record, and `false` otherwise. + `ReflectionRecord::getInlineConstructor(): ReflectionFunction` - Returns the inline constructor of the record as `ReflectionFunction`. + `ReflectionRecord::getTraditionalConstructor(): ReflectionMethod` - Returns the traditional constructor of the record as `ReflectionMethod`. + `ReflectionRecord::makeMutable(Record $instance): object` - Returns a new record instance with the properties mutable. + `ReflectionRecord::isMutable(Record $instance): bool` - Returns `true` if the record is mutable, and `false` otherwise.Tools
LanguageTool
[uncategorized] ~433-~433: Loose punctuation mark.
Context: ...inalizeRecord(object $instance): Record`: Finalizes a record under construction, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...onRecord::isRecord(mixed $object): bool: Returnstrue` if the object is a recor...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~435-~435: Loose punctuation mark.
Context: ...InlineConstructor(): ReflectionFunction`: Returns the inline constructor of the r...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~436-~436: Loose punctuation mark.
Context: ...ditionalConstructor(): ReflectionMethod`: Returns the traditional constructor of ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~437-~437: Loose punctuation mark.
Context: ...::makeMutable(Record $instance): object`: Returns a new record instance with the ...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~437-~437: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...e(Record $instance): object`: Returns a new record instance with the properties mutable. -...(EN_WORDINESS_PREMIUM_NEW_RECORD)
[uncategorized] ~438-~438: Loose punctuation mark.
Context: ...cord::isMutable(Record $instance): bool: Returnstrue` if the record is mutable...(UNLIKELY_OPENING_PUNCTUATION)
525-526: Simplify wording.Consider removing "currently" to improve readability.
[style]- PHP doesn’t currently support autoloading functions, + PHP doesn’t support autoloading functions,
566-566: Fix misspelling.The term "Todo" should be spelled as "To-do".
[misspelling]- Todo + To-do
46-46: Clarify the example context.Consider rephrasing the example context for clarity.
- Let’s take a look at the updated example, using a `record` type for `UserId`. + Let’s take a look at an updated example using a `record` type for `UserId`.Likely invalid or redundant comment.
249-251: Clarify the property initialization.Consider rephrasing for clarity.
- the properties are already initialized to the values from the inline constructor - and are mutable until the end of the method, - at which point they become immutable. + the properties are already initialized to the values from the inline constructor and are mutable until the end of the method, at which point they become immutable.Likely invalid or redundant comment.
1-1: Consider adding a title.To improve clarity, consider adding a title to the document.
+ # PHP RFC: RecordsLikely invalid or redundant comment.
242-242: Clarify the types of constructors.Consider rephrasing for clarity.
- A **record** has two types of constructors: the inline constructor and the traditional constructor. + A **record** has two types of constructors: the inline constructor and the traditional constructor.Likely invalid or redundant comment.
|
@coderabbitai pause |
Actions performedReviews paused. |
|
Implementation update:
Working on compilation now. |
2 participants
This is an RFC for immutable data structures.
Todo:
ReflectionRecordclass that inherits fromReflectionClass(no calling constructor)Summary by CodeRabbit
Summary by CodeRabbit
New Features
recordobjects in PHP for creating immutable data structures, simplifying the process of managing data objects.withmethod for partial updates while maintaining immutability.Configuration Changes
These changes aim to enhance usability, reduce boilerplate code, and improve the feedback process for developers.