Skip to content

Revert "Do not call close..." and release 0.9.16#773

Merged
fredrikekre merged 2 commits intomasterfrom
fe/0.9.16
Sep 29, 2021
Merged

Revert "Do not call close..." and release 0.9.16#773
fredrikekre merged 2 commits intomasterfrom
fe/0.9.16

Conversation

@fredrikekre
Copy link
Copy Markdown
Member

@fredrikekre fredrikekre commented Sep 29, 2021

Since #752 turned out to be rather breaking I propose a new release with that change reverted. We should also yank 0.9.15 from the registry.

Fixes #772

cc @oxinabox @mattBrzezinski @nickrobinson251 @c42f @quinnj @omus

Copy link
Copy Markdown
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

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

I approve this.
To be clear I think it is entirely possible that #752 is the correct thing to do.
But I think we would like have a bit more time to think about it,
and reverting it and tagging a patch gives us that time.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 29, 2021

Codecov Report

Merging #773 (3621089) into master (52948e7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 3621089 differs from pull request most recent head 3e17e94. Consider uploading reports for the commit 3e17e94 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #773   +/-   ##
=======================================
  Coverage   77.46%   77.47%           
=======================================
  Files          38       38           
  Lines        2561     2562    +1     
=======================================
+ Hits         1984     1985    +1     
  Misses        577      577           
Impacted Files Coverage Δ
src/StreamRequest.jl 95.52% <100.00%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52948e7...3e17e94. Read the comment docs.

Copy link
Copy Markdown
Contributor

@omus omus left a comment

Choose a reason for hiding this comment

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

I'm okay with reverting this change as long as we create an issue for removing close as part of a breaking release and mention it in the change logs.

@oxinabox
Copy link
Copy Markdown
Member

long as we create an issue for removing close as part of a breaking release and mention it in the change logs.

That is one possibility, but let's take the time to discuss it and think it through fully.
Which can wait til we open that issue.

@omus
Copy link
Copy Markdown
Contributor

omus commented Sep 29, 2021

That is one possibility, but let's take the time to discuss it and think it through fully. Which can wait til we open that issue.

Basically I want to make that issue now

@c42f
Copy link
Copy Markdown
Contributor

c42f commented Oct 5, 2021

This is fine by me in terms of keeping the stable releases going.

I still think the original patch was the correct approach, so I'll propose it again as a PR, and we can mark it breaking, and decide when to merge it.

@c42f
Copy link
Copy Markdown
Contributor

c42f commented Oct 5, 2021

See #775

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.

Requests to AWS S3 hang with HTTP.jl v0.9.15

5 participants