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

Use cache folder for db_last_error #4305

Merged
merged 1 commit into from Jan 26, 2018

Conversation

Projects
None yet
3 participants
@jdarwood007
Member

jdarwood007 commented Sep 16, 2017

Also, Make sure we only verify minified files are writable if we have it enabled or have not set it yet.

Not all forums run with having world writable files in public folders. This changes this to use the cache folder. Which should be writable and per a admins configuration can be outside of the any public folders.

The only thing this doesn't cover it cleaning up Settings.php. But so far none of the other changes done also cleanup Settings.php or prep it for a improved upgraded version.

Signed-off-by: jdarwood007 unmonitored+github@sleepycode.com

Move db_last_error.php to the cache directory, its safer to have it w…
…orld writable than the root SMF folder.

Make sure we only verify minified files are writable if we have it enabled or have not set it yet.

Signed-off-by: jdarwood007 <unmonitored+github@sleepycode.com>
@Oldiesmann

This comment has been minimized.

Show comment
Hide comment
@Oldiesmann

Oldiesmann Sep 17, 2017

Member

From a support standpoint it's best to change Settings.php on upgrade as well. Otherwise it gets too confusing and people who upgrade from previous versions of SMF won't be able to take advantage of features such as this.

Member

Oldiesmann commented Sep 17, 2017

From a support standpoint it's best to change Settings.php on upgrade as well. Otherwise it gets too confusing and people who upgrade from previous versions of SMF won't be able to take advantage of features such as this.

@Sesquipedalian

This comment has been minimized.

Show comment
Hide comment
@Sesquipedalian

Sesquipedalian Oct 20, 2017

Member

The only thing this doesn't cover it cleaning up Settings.php. But so far none of the other changes done also cleanup Settings.php or prep it for a improved upgraded version.

Would you be willing to write some code to do that in the upgrader? Perhaps as part of another PR so that we can merge this one in the meantime.

Member

Sesquipedalian commented Oct 20, 2017

The only thing this doesn't cover it cleaning up Settings.php. But so far none of the other changes done also cleanup Settings.php or prep it for a improved upgraded version.

Would you be willing to write some code to do that in the upgrader? Perhaps as part of another PR so that we can merge this one in the meantime.

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Oct 20, 2017

Member

I could look into that. Would it be too late in this stage to do that?

The only way I see this being possible is having the Settings.php as a template in the upgrader, injecting the updated variables into said template, then testing to make sure the variables match between the current and new one, then finally writing the file.

Member

jdarwood007 commented Oct 20, 2017

I could look into that. Would it be too late in this stage to do that?

The only way I see this being possible is having the Settings.php as a template in the upgrader, injecting the updated variables into said template, then testing to make sure the variables match between the current and new one, then finally writing the file.

@Oldiesmann

This comment has been minimized.

Show comment
Hide comment
@Oldiesmann

Oldiesmann Nov 4, 2017

Member

Can't we just use the existing functionality for updating Settings.php?

Member

Oldiesmann commented Nov 4, 2017

Can't we just use the existing functionality for updating Settings.php?

@jdarwood007

This comment has been minimized.

Show comment
Hide comment
@jdarwood007

jdarwood007 Nov 4, 2017

Member

There isn't any template, the existing updater just updates vars and/or adds them if missing to the end.

Member

jdarwood007 commented Nov 4, 2017

There isn't any template, the existing updater just updates vars and/or adds them if missing to the end.

@albertlast albertlast referenced this pull request Jan 14, 2018

Open

RC1 Checklist #4485

10 of 13 tasks complete
@Sesquipedalian

This comment has been minimized.

Show comment
Hide comment
@Sesquipedalian

Sesquipedalian Jan 26, 2018

Member

The only thing this doesn't cover it cleaning up Settings.php.

I've put together some code to do this now that seems to work nicely. So I'm going to merge this now and then submit a PR with those further changes.

Member

Sesquipedalian commented Jan 26, 2018

The only thing this doesn't cover it cleaning up Settings.php.

I've put together some code to do this now that seems to work nicely. So I'm going to merge this now and then submit a PR with those further changes.

@Sesquipedalian Sesquipedalian merged commit a92a58e into SimpleMachines:release-2.1 Jan 26, 2018

2 checks passed

Scrutinizer 1 new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jdarwood007 jdarwood007 deleted the jdarwood007:db_last_error branch May 2, 2018

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