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

Fix unsetMethod in ModelsCommand #1453

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Fix unsetMethod in ModelsCommand #1453

merged 3 commits into from
Feb 7, 2024

Conversation

leo108
Copy link
Contributor

@leo108 leo108 commented Jul 18, 2023

Summary

The key of ModelsCommand::$methods is case sensitive, so we should not convert the $name to lowercase when unset.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@leo108 leo108 force-pushed the patch-1 branch 2 times, most recently from 04032ba to 22fddc9 Compare July 18, 2023 14:25
@leo108
Copy link
Contributor Author

leo108 commented Aug 7, 2023

@barryvdh any chance to check this PR? thanks.

@barryvdh
Copy link
Owner

barryvdh commented Aug 7, 2023

Maybe we should lowercase the check instead also?

@leo108
Copy link
Contributor Author

leo108 commented Aug 7, 2023

@barryvdh You're right since methods are case-insensitive in PHP.

Please check the updated code.

@leo108
Copy link
Contributor Author

leo108 commented Aug 15, 2023

@barryvdh Please check when you have time, thanks.

@leo108
Copy link
Contributor Author

leo108 commented Aug 21, 2023

@barryvdh bump

@leo108
Copy link
Contributor Author

leo108 commented Dec 2, 2023

@barryvdh Sorry to bother you again, but I'm really hope this PR can be merged, please let me know what your concern, thanks!

@barryvdh barryvdh closed this Feb 7, 2024
@barryvdh barryvdh reopened this Feb 7, 2024
@barryvdh
Copy link
Owner

barryvdh commented Feb 7, 2024

Hmm not really sure why this is failing

@barryvdh
Copy link
Owner

barryvdh commented Feb 7, 2024

ah probably unrelated, let me try again.

@barryvdh barryvdh closed this Feb 7, 2024
@barryvdh barryvdh reopened this Feb 7, 2024
@barryvdh barryvdh merged commit 409bbf6 into barryvdh:master Feb 7, 2024
40 of 66 checks passed
@barryvdh
Copy link
Owner

barryvdh commented Feb 7, 2024

All good! Thanks for your patience

@leo108 leo108 deleted the patch-1 branch February 8, 2024 02:14
@leo108
Copy link
Contributor Author

leo108 commented Feb 8, 2024

Thanks! it would be nice if you could release a new version.

d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
* Fix unsetMethod in ModelsCommand

* composer fix-style

---------

Co-authored-by: Barry vd. Heuvel <[email protected]>
Co-authored-by: laravel-ide-helper <[email protected]>
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.

2 participants