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

Feature/php8.2 deprecations #36

Closed

Conversation

bartvanhoutte
Copy link
Contributor

@bartvanhoutte bartvanhoutte commented Dec 15, 2022

PR as discussed in #35.

Copy link
Collaborator

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

@bartvanhoutte Thanks for opening this PR, looks great so far 👍

I added some remarks below, can you also squash your commits into one, I don't think we need that many.

public $pending;

public $args;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: missing newline

src/State.php Outdated
Comment on lines 3 to 4
namespace Clue\React\Mq;
final class State
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
namespace Clue\React\Mq;
final class State
namespace Clue\React\Mq;
/** @internal */
final class State

Just added some white space and I think we should mark this class as internal

@SimonFrings
Copy link
Collaborator

And should we give the State class a better name?

@clue clue added this to the v1.6.0 milestone Feb 20, 2023
@clue clue closed this in #43 Jul 27, 2023
@clue clue removed this from the v1.6.0 milestone Jul 28, 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.

None yet

3 participants