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.

Sign up for GitHub

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

Jump to bottom

Add RouteArgument attribute for Yii Hydrator #203

Merged
merged 14 commits into from
Jan 29, 2024
Merged

Add RouteArgument attribute for Yii Hydrator #203

merged 14 commits into from
Jan 29, 2024

Conversation

vjik
Copy link
Member

@vjik vjik commented May 15, 2023

Q A
Is bugfix? ❌
New feature? ✔️
Breaks BC? ❌
Fixed issues -

@what-the-diff
Copy link

what-the-diff bot commented May 15, 2023

PR Summary

  • Added 'Route' attribute for Yii Hydrator
    Introduces a new 'Route' attribute to improve the Yii Hydrator functionality
  • Updated dependency on yiisoft/hydrator
    Upgrades the dependency to the latest development version for better compatibility and performance

@vjik vjik changed the title Added Route attribute for Yii Hydrator Add Route attribute for Yii Hydrator May 15, 2023
@codecov
Copy link

codecov bot commented May 15, 2023 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cd98ba6) 95.54% compared to head (39e3dad) 95.66%.
Report is 1 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff              @@
##             master     #203      +/-   ##
============================================
+ Coverage     95.54%   95.66%   +0.12%     
- Complexity      174      181       +7     
============================================
  Files            14       16       +2     
  Lines           561      577      +16     
============================================
+ Hits            536      552      +16     
  Misses           25       25

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vjik vjik marked this pull request as ready for review May 16, 2023 07:18
@vjik vjik requested a review from a team May 16, 2023 07:18
@vjik vjik added the status:code review The pull request needs review. label May 16, 2023
samdark
samdark approved these changes May 16, 2023
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@vjik @samdark
Update composer.json
65140b4 
Co-authored-by: Alexander Makarov <[email protected]>
xepozz
xepozz requested changes May 16, 2023
View reviewed changes
Copy link
Member

@xepozz xepozz left a comment

Choose a reason for hiding this comment

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

Route is a wrong name for this kind of attribute.

Route is already reserved for routing: the class and coming attribute as well.
I suggest you think about RouteAttribute or something similar. Do not confuse developers with one more Route entity.

@vjik vjik requested a review from xepozz May 16, 2023 13:38
@vjik vjik changed the title Add Route attribute for Yii Hydrator Add RouteArgument attribute for Yii Hydrator May 16, 2023
@vjik vjik added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Jun 6, 2023
@vjik vjik mentioned this pull request Oct 8, 2023
Closed
@xepozz
Copy link
Member

xepozz commented Oct 14, 2023

Why is it argument, isn't parameter better?

@vjik
Copy link
Member Author

vjik commented Nov 10, 2023

Why is it argument, isn't parameter better?

In router, this is called "arguments":

router/src/CurrentRoute.php

Line 112 in 0892cbe

public function getArguments(): array

@vjik vjik added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Jan 28, 2024
@vjik vjik requested a review from a team January 28, 2024 17:48
samdark
samdark approved these changes Jan 29, 2024
View reviewed changes
composer.json Outdated Show resolved Hide resolved
@samdark samdark merged commit f3f1091 into master Jan 29, 2024
20 of 26 checks passed
@samdark samdark deleted the route-attr branch January 29, 2024 17:10
@vjik vjik mentioned this pull request Jan 30, 2024
Closed
vjik added a commit that referenced this pull request Feb 20, 2024
@vjik
Add RouteArgument attribute for Yii Hydrator (#203)
931f8b3 
(cherry picked from commit f3f1091)
vjik added a commit that referenced this pull request Feb 20, 2024
@vjik
Add RouteArgument attribute for Yii Hydrator (#237)
1a8af5c 
* Add `RouteArgument` attribute for Yii Hydrator (#203)

(cherry picked from commit f3f1091)

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samdark samdark samdark approved these changes

@xepozz xepozz Awaiting requested review from xepozz

Assignees
No one assigned
Labels
status:code review The pull request needs review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vjik @xepozz @samdark