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

Minor documentation improvements for references to deprecated Factory #241

Merged
merged 1 commit into from
Nov 13, 2021

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Oct 20, 2021

the readme should not recommend to use the factory anymore but Loop::create

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@dbu
Copy link
Contributor Author

dbu commented Oct 20, 2021

no problems, stumbled over it when somebody changed what we do in php-http/react-adapter

i accepted your changes and fixed a spelling error i noticed.

@SimonFrings
Copy link
Member

Thx for the update 👍. The only thing I have to note is that I don't think it's necessary to have three commits for these changes, maybe you could squash these three together?

@dbu
Copy link
Contributor Author

dbu commented Oct 20, 2021

of course. just squashed the commits

@SimonFrings
Copy link
Member

Looks good, nice work 👍

Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@dbu Thank you for spotting and filing this PR, the suggested changes look good to me! 👍

The same text is also used in the referenced classes (also note the maximum line length). May I ask you to amend your changes to keep this in sync?

the readme should not recommend to use the factory anymore but Loop::create

Co-authored-by: Simon Frings <simon.frings1@web.de>
@dbu
Copy link
Contributor Author

dbu commented Oct 22, 2021

i adjusted the phpdoc accordingly and made the lines shorter.

@clue clue added this to the v1.3.0 milestone Nov 13, 2021
@clue clue changed the title fix references to the deprecated factory Minor documentation improvements for references to deprecated Factory Nov 13, 2021
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@dbu Thanks for looking into this, changes LGTM! :shipit:

@WyriHaximus WyriHaximus merged commit 1d999e1 into reactphp:master Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants