Skip to content

Fixed a crash caused by Local::copyFromStorage() not conforming to Co… #5072

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
Merged
MorrisJobke merged 1 commit into from
Jun 13, 2017
Merged

Fixed a crash caused by Local::copyFromStorage() not conforming to Co… #5072

MorrisJobke merged 1 commit into from
Jun 13, 2017

Conversation

@mjmunger
Copy link
Contributor

@mjmunger mjmunger commented May 23, 2017 •
edited by nickvergessen
Loading

Fix #5071
Local::copyFromStorage was not adhering to Common::copyFromStorage or an interface that governs the two of them. Added the parameter to make it conform.

@mention-bot
Copy link

mention-bot commented May 23, 2017

@mjmunger, thanks for your PR! By analyzing the history of the files in this pull request, we identified @icewind1991, @nickvergessen and @LukasReschke to be potential reviewers.

icewind1991
icewind1991 approved these changes May 23, 2017
View reviewed changes
Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

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

Makes sense

@mjmunger
Copy link
Contributor Author

mjmunger commented May 24, 2017

Not sure if you need anything else from me?

@ChristophWurst ChristophWurst mentioned this pull request May 29, 2017
Closed
@nickvergessen
Copy link
Member

nickvergessen commented May 30, 2017

@mjmunger can you fix lib/private/Lockdown/Filesystem/NullStorage.php as well?

Other then that it looks good.

@MorrisJobke MorrisJobke added this to the Nextcloud 13 milestone Jun 13, 2017
@MorrisJobke
Copy link
Member

MorrisJobke commented Jun 13, 2017

let's get this in and fix the NullStorage separately.

@MorrisJobke MorrisJobke merged commit 0b92700 into nextcloud:master Jun 13, 2017
@mjmunger
Copy link
Contributor Author

mjmunger commented Jun 13, 2017

Same issue with lib/private/Lockdown/Filesystem/NullStorage.php? Or something different?

@MorrisJobke
Copy link
Member

MorrisJobke commented Jun 13, 2017

Same issue with lib/private/Lockdown/Filesystem/NullStorage.php? Or something different?

Yes, it seems so - do you create a PR to fix this as well or should I?

@MorrisJobke MorrisJobke mentioned this pull request Jun 13, 2017
Merged
@MorrisJobke
Copy link
Member

MorrisJobke commented Jun 13, 2017

Backport of this is in #5397

@mjmunger
Copy link
Contributor Author

mjmunger commented Jun 13, 2017

Create a PR and assign to me. Im not at a computer ATM.

@mjmunger
Copy link
Contributor Author

mjmunger commented Jun 13, 2017

Apparently, I'm drunk. You can't assign a pull request. Please create it. I'm not at a computer.

@MorrisJobke
Copy link
Member

MorrisJobke commented Jun 14, 2017

@mjmunger can you fix lib/private/Lockdown/Filesystem/NullStorage.php as well?

Just checked and the NullStorage doesn't seem to need it. So will keep it as it is. Thanks anyway @mjmunger

@Aterfax Aterfax mentioned this pull request Sep 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Nextcloud 13

Development

Successfully merging this pull request may close these issues.

5 participants

@mjmunger @mention-bot @nickvergessen @MorrisJobke @icewind1991