Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Bump net-http-persistent to 3.1.0 #7200

Merged
5 commits merged into from
Jul 25, 2019
Merged

Bump net-http-persistent to 3.1.0 #7200

5 commits merged into from
Jul 25, 2019

Conversation

deivid-rodriguez
Copy link
Member

What was the end-user problem that led to this PR?

The problem was that I wanted to propose some changes to this vendored gem, but I found out that we are using an old version of it.

What was your diagnosis of the problem?

My diagnosis was that we should upgrade.

What is your fix for the problem, implemented in this PR?

My fix is to upgrade the dependency. Since it's a major update, it required some changes. Also, I had to:

@colby-swandale
Copy link
Member

I'm ✅ on upgrading the gem but i would much prefer we have the open PR in net-http-presistent merged first before merging this.

@deivid-rodriguez
Copy link
Member Author

Yeah. It's been ignored for two years now, though... 😬

@colby-swandale
Copy link
Member

Looking into the current status with drbrain/net-http-persistent#90. We probably don't have a choice, and the PR is fairly simple that i doubt it will cause an issue. So i'm 👍 on this.

Can we just document (even just a comment in the Rakefile) that we've cherry-picked this change incase we need to reference it in the future.

@deivid-rodriguez
Copy link
Member Author

A comment about the cherry-pick sounds good, yeah. I'll add it 👍.

@deivid-rodriguez
Copy link
Member Author

Another downside of this PR is that the automatiek task for vendoring net-http-persistent now needs manual intervention because its connection_pool requires need to be edited to point to the vendored version.

@deivid-rodriguez
Copy link
Member Author

I'll try to fix the last issue I mentioned in automatiek.

@deivid-rodriguez
Copy link
Member Author

I created a patch for automatiek and described in the PR's description how it would be used for this specific case: segiddins/automatiek#3.

Rakefile Outdated
# We currently cherry-pick changes to use `require_relative` internally
# instead of regular `require`. These changes have already landed to fileutils
# master, but have not yet been released, so we cherry-pick them for the time
# being.
Copy link
Member Author

Choose a reason for hiding this comment

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

@hsbt If you have time, it would be nice to release fileutils master as fileutils-1.2.1, so we don't need to cherry-pick anything.

* Adds an extra artifice task to vendorize new `connection_pool`
dependency.

* Cherry-pick's needed Windows fix not yet merged into master branch of
`net-http-persistent`.

* Update bundler usages to be compatible with the new version, and fix
unit specs.
@deivid-rodriguez
Copy link
Member Author

The net-http-persistent PR has been merged! Maybe we'll be able to jump straight to the next release 🎉

@deivid-rodriguez deivid-rodriguez changed the title Bump net-http-persistent to 3.0.1 Bump net-http-persistent to 3.1.0 Jul 25, 2019
@deivid-rodriguez
Copy link
Member Author

Updated to use net-http-persistent 3.1.0, which was released yesterday, including the PR we needed.

Only thing left here would be an automatiek release including segiddins/automatiek#3, so that net-http-persistent can be vendored without manual intervention.

But I don't think it's a hard requirement for this PR.

@deivid-rodriguez
Copy link
Member Author

This was approved by @hbst and @colby-swandale, so I'll merge it.

Vendoring a future version of net-http-persistent is not automatic, but almost (only connection_pool references need manual intervention).

So no big deal, we can wait for the PR to be merged and reviewed, and update automatiek later.

@bundlerbot r+

ghost pushed a commit that referenced this pull request Jul 25, 2019
7200: Bump net-http-persistent to 3.1.0 r=deivid-rodriguez a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that I wanted to propose some changes to this vendored gem, but I found out that we are using an old version of it.

### What was your diagnosis of the problem?

My diagnosis was that we should upgrade.

### What is your fix for the problem, implemented in this PR?

My fix is to upgrade the dependency. Since it's a major update, it required some changes. Also, I had to:

* Add a new artifice task to vendorize new `connection_pool` dependency. This is the main downside of this PR, that the new version adds a dependency on this gem. But this gem is very stable, and rarely changes and releases new versions, as can be seen by its [releases](https://github.com/mperham/connection_pool/releases). 

* Cherry-pick a Windows fix not yet merged into master branch of `net-http-persistent`: drbrain/net-http-persistent#90.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@deivid-rodriguez
Copy link
Member Author

Actually, the comment about thor is incorrect, let me fix it.

@bundlerbot r-

@ghost
Copy link

ghost commented Jul 25, 2019

Canceled

@deivid-rodriguez
Copy link
Member Author

Done, and also added a comment about the current automatiek situation.

@bundlerbot r+

ghost pushed a commit that referenced this pull request Jul 25, 2019
7200: Bump net-http-persistent to 3.1.0 r=deivid-rodriguez a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that I wanted to propose some changes to this vendored gem, but I found out that we are using an old version of it.

### What was your diagnosis of the problem?

My diagnosis was that we should upgrade.

### What is your fix for the problem, implemented in this PR?

My fix is to upgrade the dependency. Since it's a major update, it required some changes. Also, I had to:

* Add a new artifice task to vendorize new `connection_pool` dependency. This is the main downside of this PR, that the new version adds a dependency on this gem. But this gem is very stable, and rarely changes and releases new versions, as can be seen by its [releases](https://github.com/mperham/connection_pool/releases). 

* Cherry-pick a Windows fix not yet merged into master branch of `net-http-persistent`: drbrain/net-http-persistent#90.

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@ghost
Copy link

ghost commented Jul 25, 2019

Build succeeded

@ghost ghost merged commit ffb7d6f into master Jul 25, 2019
@ghost ghost deleted the bump_net_http_persistent branch July 25, 2019 21:50
deivid-rodriguez added a commit that referenced this pull request Aug 3, 2019
This reverts commit 32c01e4, reversing
changes made to 75db5f5.
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants