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

Code modernization #207

Open
wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Code modernization #207

wants to merge 32 commits into from

Conversation

aon4o
Copy link

@aon4o aon4o commented Oct 10, 2024

I tried to modernize the library's source code in the following pull request.

Why?

I needed to extend the ImplersonateManager code in a given project. I had no problems doing this, but the project implies 100% code coverage, which was impossible due to the extension of the manager. It has no argument types, so when extending, I can't put them in either.

What have I done?

  • I have dropped support for PHP and Laravel versions that are no longer supported;
  • Migrated the test suite to use the Pest testing framework;
  • Added test-coverage and type-coverage reports;
  • Updated the GitHub Workflow accordingly.

Hope this PR helps and gets merged 😃

Copy link

@RelBreR RelBreR left a comment

Choose a reason for hiding this comment

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

All tests pass - I think that an update was really needed

@aglipanci
Copy link

aglipanci commented Dec 9, 2024

This looks really good! Hopefully, it's merged.

@aon4o aon4o requested a review from RelBreR December 18, 2024 07:53
Copy link

@RelBreR RelBreR left a comment

Choose a reason for hiding this comment

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

I really hope this gets merged. All looks good

@MarceauKa
Copy link
Member

Let me review this :)

@MarceauKa MarceauKa mentioned this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants