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.

Sign up for GitHub

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

Jump to bottom

Prepare release #149

Merged
merged 10 commits into from
Oct 31, 2018
Merged

Prepare release #149

merged 10 commits into from
Oct 31, 2018

Conversation

sagikazarmark
Copy link
Member

@sagikazarmark sagikazarmark commented Oct 29, 2018 •
edited
Loading

Closes #139

@sagikazarmark sagikazarmark requested a review from Nyholm October 29, 2018 11:28
@sagikazarmark sagikazarmark requested a review from dbu October 29, 2018 11:28
@sagikazarmark sagikazarmark mentioned this pull request Oct 29, 2018
Closed
dbu
dbu reviewed Oct 29, 2018
View reviewed changes
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks. commented on some details.

README.md Outdated
@@ -13,6 +13,15 @@
**HTTPlug, the HTTP client abstraction for PHP.**


## Intro

HTTPlug served as a reference is the predecessor of [PSR-18](https://github.com/php-fig/fig-standards/blob/master/accepted/http-client/http-client.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/served as a reference//

README.md
## Intro

This is the contract package for HTTP Client.
Use it to create HTTP Clients which are interoperable and compatible with [PSR-7](http://www.php-fig.org/psr/psr-7/).
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd love to keep mentioning PSR-7 somewhere in the intro

composer.json
},
"extra": {
"branch-alias": {
"dev-2.x": "2.0.x-dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

we should alias master to 2.x-dev when this is merged.

README.md Outdated

HTTPlug served as a reference is the predecessor of [PSR-18](https://github.com/php-fig/fig-standards/blob/master/accepted/http-client/http-client.md)
HTTP Client standard. Since there is an entire ecosystem built around HTTPlug and since it's already widely adapted,
this package stays available, but new implementations and consumers should use the PSR-18 interfaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

could we say instead "we will keep maintaining this package for the time being, but new ..."

README.md Outdated
@@ -13,6 +13,15 @@
**HTTPlug, the HTTP client abstraction for PHP.**


## Intro

HTTPlug served as a reference is the predecessor of [PSR-18](https://github.com/php-fig/fig-standards/blob/master/accepted/http-client/http-client.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

and we should mention that version 2 of httplug extends the psr-18 interfaces to allow a convenient migration path.

@sagikazarmark
Copy link
Member Author

Thanks for the review @dbu, I made the requested changes.

dbu
dbu reviewed Oct 29, 2018
View reviewed changes
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

sorry, some further nitpicks.

README.md Outdated

HTTPlug is the predecessor of [PSR-18](http://www.php-fig.org/psr/psr-18/)
HTTP Client standard built on [PSR-7](http://www.php-fig.org/psr/psr-7/) HTTP messages.
Since there is an entire ecosystem built around HTTPlug which is already widely adapted,
Copy link
Contributor

Choose a reason for hiding this comment

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

s/adapted/adopted/

README.md Outdated
we will keep maintaining this package for the time being,
but new implementations and consumers should use the PSR-18 interfaces.
HTTPlug 2.x extends the PSR-18 interfaces to allow a convenient migration path,
but in time it may be completely replaced by the standard.
Copy link
Contributor

Choose a reason for hiding this comment

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

i would put .on the line above and then say "In the long term, we expect PSR-18 to completely replace the need for Httplug."

dbu
dbu approved these changes Oct 30, 2018
View reviewed changes
Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks!

@dbu
Copy link
Contributor

dbu commented Oct 30, 2018

now we need to wait for psr-18 to be tagged 1.0.0, i guess?

@sagikazarmark
Copy link
Member Author

Yeah. I think @Nyholm has access to it from our side, but I don't think there are any blockers there.

Nyholm
Nyholm approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Excellent

@Nyholm Nyholm merged commit b384253 into master Oct 31, 2018
@sagikazarmark sagikazarmark deleted the prepare_release branch October 31, 2018 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nyholm Nyholm Nyholm approved these changes

@dbu dbu dbu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sagikazarmark @dbu @Nyholm