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

Replace uploaddir with datadir #1272

Merged
merged 1 commit into from Jan 30, 2017
Merged

Replace uploaddir with datadir #1272

merged 1 commit into from Jan 30, 2017

Conversation

strugee
Copy link
Member

@strugee strugee commented Jan 21, 2017

As discussed in #1261, deprecate uploaddir and replace it with datadir and uploadsEnabled. Anyone who hates the new names, speak now or forever hold your peace.

@strugee
Copy link
Member Author

strugee commented Jan 21, 2017

@evanp if you have time I'd love a review given that this is semver-major.

@strugee strugee requested a review from evanp January 21, 2017 17:14
@strugee strugee merged commit 11ee851 into master Jan 30, 2017
@evanp
Copy link
Contributor

evanp commented Mar 23, 2017

I HATE THE NEW NAMES.

@strugee
Copy link
Member Author

strugee commented Mar 23, 2017

@evanp can't tell if you're kidding or not? We can change them but it'd mean another semver-major release

@evanp
Copy link
Contributor

evanp commented Mar 23, 2017

Mostly kidding. I feel like dealing with the deprecation was a pain for me as an admin, and since we don't yet use datadir for anything else except uploads, it wasn't clear what the value was for me as an admin.

I can live with it now, but maybe we could just show a warning if 'uploaddir' is set, instead of aborting?

@strugee
Copy link
Member Author

strugee commented Mar 23, 2017

@evanp ah, okay haha!

Your logic totally makes sense - originally I'd planned to merge some stuff that made use of the change in the same cycle, but ran out of time. I'm uncomfortable doing automatic migrations like this in core (just because it's dangerous and may leave the node broken with no warning) but in the future I'll try to provide better automation, and I'll be uploading a script to perform this that admins can run instead of doing it themselves.

That should alleviate the need to warn instead of abort, which I'd rather not do since 3.0 has been out for a while already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants