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

Added config for file_put_contents flags #507

Merged
merged 1 commit into from Jun 13, 2015
Merged

Added config for file_put_contents flags #507

merged 1 commit into from Jun 13, 2015

Conversation

nathan-van-der-werf
Copy link

For a project we encountered a problem with the local adapter, our mounted storage (nfs share) didn't support exclusive locking.

In the pull request you can set the flags for the file_put_contents in the config.

@GrahamCampbell
Copy link
Member

👎 as this config isn't available in all adapters. It's WRONG to call the write method with config specific to the adapter in question.

@GrahamCampbell
Copy link
Member

Maybe we need to make this locking configurable for the entire adapter instance?

@nathan-van-der-werf
Copy link
Author

I pushed a new approach for the flags in the local adapter.

@frankdejonge
Copy link
Member

I'd say making it a constructor arguments would make this better. Agree @GrahamCampbell ?

@GrahamCampbell
Copy link
Member

Yeh, that's what I was meaning @frankdejonge. :)

* @param int $flags
* @return $this
*/
public function setFlags($flags)
Copy link
Member

Choose a reason for hiding this comment

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

👎 I think this should be immutable. Should be configured on adapter instantiation.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, I unfortunately made this mistake for some other settings (like in the ftp stuff) and I don't like it.

@nathan-van-der-werf
Copy link
Author

Like this ? :)

@frankdejonge
Copy link
Member

Indeed, like that. Now I think we need to get a better name for this. Possibly writeFlags. Suggestions @GrahamCampbell ?

@GrahamCampbell
Copy link
Member

Either is fine IMO @frankdejonge. :)

@frankdejonge
Copy link
Member

@dogiedog can you update the param name to writeFlags and squash the commits?

@nathan-van-der-werf
Copy link
Author

Done

@frankdejonge
Copy link
Member

@dogiedog can you squash the commits too?

@nathan-van-der-werf
Copy link
Author

Done

frankdejonge added a commit that referenced this pull request Jun 13, 2015
Added config for file_put_contents flags
@frankdejonge frankdejonge merged commit 30539a6 into thephpleague:master Jun 13, 2015
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.

None yet

3 participants