-
Notifications
You must be signed in to change notification settings - Fork 16
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: migration broke in laravel v11.15 #224
Conversation
WalkthroughThe recent changes introduce version 8.2.0, raising the minimum supported Laravel version to 11.15.0 and addressing a significant bug related to schema changes being applied multiple times. Improvements to the SQL compilation logic enhance both flexibility and efficiency. Additionally, a deprecation notice for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Application
participant Database
User->>Application: Request to change schema
Application->>Database: Compile change request
Database->>Application: Return compiled SQL
Application->>User: Confirm change applied
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
CHANGELOG.md (1)
4-4
: Fix repeated word.There is a repeated word "Fixed" in the changelog entry.
- Fixed - Fixed an issue where Schema changes were applied twice. (#224) + - Fixed an issue where Schema changes were applied twice. (#224)Tools
LanguageTool
[duplication] ~4-~4: Possible typo: you repeated a word
Context: ...version is bumped to 11.15.0 for #224. Fixed - Fixed an issue where Schema changes were appl...(ENGLISH_WORD_REPEAT_RULE)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- composer.json (1 hunks)
- src/Schema/Grammar.php (2 hunks)
Additional context used
LanguageTool
CHANGELOG.md
[duplication] ~4-~4: Possible typo: you repeated a word
Context: ...version is bumped to 11.15.0 for #224. Fixed - Fixed an issue where Schema changes were appl...(ENGLISH_WORD_REPEAT_RULE)
Additional comments not posted (4)
composer.json (1)
14-14
: LGTM! Version update for Laravel framework.The version constraint for
laravel/framework
has been updated to^11.15.0
. This ensures compatibility with Laravel version 11.15.0 and above.CHANGELOG.md (1)
1-6
: LGTM! Changelog entry for version 8.2.0.The changelog entry is clear and informative, detailing the minimum Laravel version requirement and a critical bug fix.
Tools
LanguageTool
[duplication] ~4-~4: Possible typo: you repeated a word
Context: ...version is bumped to 11.15.0 for #224. Fixed - Fixed an issue where Schema changes were appl...(ENGLISH_WORD_REPEAT_RULE)
src/Schema/Grammar.php (2)
838-839
: Deprecation notice is clear.The deprecation notice for the
formatTimestampValue
method is clear and helpful for future developers.
134-146
: LGTM! Changes tocompileChange
method.The changes to the
compileChange
method improve flexibility and streamline SQL generation. The updated return type and logic are clear and efficient.However, ensure that all usages of the
compileChange
method are compatible with the new return type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
CHANGELOG.md (1)
3-3
: Clarify the Note on Laravel VersionConsider rephrasing for a more formal tone.
- > [!NOTE] Minimum supported Laravel version is bumped to 11.15.0 for #224. + > [!NOTE] The minimum supported Laravel version has been increased to 11.15.0 for #224.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- src/Schema/Grammar.php (3 hunks)
Additional context used
LanguageTool
CHANGELOG.md
[style] ~5-~5: Consider using a different verb for a more formal wording.
Context: ...rsion is bumped to 11.15.0 for #224. - Fixed an issue where Schema changes were appl...(FIX_RESOLVE)
Additional comments not posted (7)
CHANGELOG.md (2)
1-1
: Correct and Informative Version EntryThe version entry for 8.2.0 is correctly formatted and includes the date.
5-5
: Clear and Concise Bug Fix EntryThe bug fix description is clear and concise.
Tools
LanguageTool
[style] ~5-~5: Consider using a different verb for a more formal wording.
Context: ...rsion is bumped to 11.15.0 for #224. - Fixed an issue where Schema changes were appl...(FIX_RESOLVE)
src/Schema/Grammar.php (5)
118-118
: Accurate Return Type AnnotationThe return type annotation for
compileAdd
is accurate and enhances type safety.
122-130
: Efficient SQL Generation incompileAdd
The updated logic for SQL generation in
compileAdd
is more efficient and readable.
139-139
: Accurate Return Type AnnotationThe return type annotation for
compileChange
is accurate and enhances type safety.
143-151
: Efficient SQL Generation incompileChange
The updated logic for SQL generation in
compileChange
is more efficient and readable.
843-844
: Deprecation Notice forformatTimestampValue
The deprecation notice for
formatTimestampValue
is clear and provides adequate warning for future changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/Eloquent/Model.php (3 hunks)
Additional comments not posted (3)
src/Eloquent/Model.php (3)
27-27
: Improved type inference for static methods.The
@mixin Builder<static>
annotation enhances type inference for static methods, improving autocompletion and type checking.
45-45
: Enhanced type safety forresolveRouteBindingQuery
method.The parameter type hint now specifies that the
Relation
type is parameterized, improving type safety and understanding of what types can be passed to this method.
62-62
: Improved return type forresolveChildRouteBindingQuery
method.The return type now provides more detailed information about the type of
Relation
being returned, enhancing type safety and preventing type-related errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Broke when laravel/framework#51373 was merged.
Summary by CodeRabbit