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

Private file uploads and remove date folder requirement #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

that0n3guy
Copy link

Couldn't this be added into a config file, possibly in upload_folder_public_path?

I don't think it should be hard coded.

that0n3guy added 2 commits May 9, 2014 15:24
This should be added into a config file.
I needed to be able to sometimes upload to a "private" (not in public
folder) folder.
@that0n3guy that0n3guy changed the title Remove date folder Private file uploads and remove date folder requirement May 21, 2014
@that0n3guy
Copy link
Author

@andrewelkins what do you think about these changes?

@that0n3guy
Copy link
Author

As I look more and more at this package... I have a question..

Why specify 2 paths:

'upload_folder' => 'public/cabinet/uploads/',
'upload_folder_public_path' => 'cabinet/uploads/',

Shouldn't this be simplified and use one path?

@andrewelkins
Copy link
Owner

On upload vs public path. They are used in different spots. the folder is the pull path, whereas the public path is off of the web root.

@andrewelkins
Copy link
Owner

Why did you remove the date folder path?

@that0n3guy
Copy link
Author

It seemed odd to hard code it.

@andrewelkins
Copy link
Owner

The date folder path prevents making a single folder gigantic. Years ago it was done this way to prevent exhausting the file count limit within a folder. Now it just prevents a folder becoming so huge that it's nearly impossible to do anything useful in. eg. ls taking minutes or doing a grep/find

@olso
Copy link

olso commented Apr 21, 2015

@andrewelkins
I was actually pleasantly surprised that Cabinet creates date directory path.
It wasn't mentioned anywhere 👍

@andrewelkins
Copy link
Owner

Thanks @olso

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.

3 participants