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

Bugfix: TypeError caused by improper use of new self() instead of new static() in base class method (#878) #879

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

Quarkay
Copy link
Contributor

@Quarkay Quarkay commented Nov 1, 2023

Attempt to fix issue #878 and added simple test for validation.

Particularly, for final class DeleteResponse , replaced 'new static()' with the more appropriate 'new self()', since it's a final class.

@Quarkay Quarkay force-pushed the deal-with-type-error branch from cdec0e1 to 99c9be6 Compare November 1, 2023 07:02
@vaibhavkumar-sf
Copy link

PR Analysis

  • 🎯 Main theme: Bug fix
  • 📝 PR summary: Attempt to fix a bug causing a TypeError by replacing new self() with new static() in base class methods.
  • 📌 Type of PR: Bug fix
  • 🧪 Relevant tests added: True
  • ⏱️ Estimated effort to review [1-5]: 2, because the changes are straightforward and limited to a few files.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The bug fix seems appropriate and the added tests provide good coverage. However, it would be helpful to include a brief explanation in the PR description about why new static() is more appropriate than new self() in this context.

  • 🤖 Code feedback:

    • relevant file: src/Model/Suppression/Bounce/Bounce.php
      suggestion: Consider removing the private constructor since it is no longer necessary. [medium]
      relevant line: final private function __construct()

    • relevant file: src/Model/Suppression/Complaint/Complaint.php
      suggestion: Consider removing the private constructor since it is no longer necessary. [medium]
      relevant line: final private function __construct()

    • relevant file: src/Model/Suppression/Unsubscribe/Unsubscribe.php
      suggestion: Consider removing the private constructor since it is no longer necessary. [medium]
      relevant line: final private function __construct()

    • relevant file: src/Model/Suppression/Whitelist/DeleteResponse.php
      suggestion: Consider using new static() instead of new self() in the create method. [important]
      relevant line: - $model = new static()

@oleksandr-mykhailenko oleksandr-mykhailenko merged commit 57d5531 into mailgun:master Nov 27, 2023
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.

3 participants