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

Add temp folder check during install #3312

Merged
merged 1 commit into from Jan 2, 2021

Conversation

aledeg
Copy link
Member

@aledeg aledeg commented Jan 1, 2021

Closes #3310

Changes proposed in this pull request:

  • Add temp folder check during install

How to test the feature manually:

  1. Make a new install with a writable temp folder.
  2. Make a new install with a non-writable temp folder.

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

Additional information can be found in the documentation.

Before, the temp path was not check during install. With some configuration,
FRSS was not working because of a non-writable temp directory. It happened
with XAMPP on MacOS X but it might be the case for other platforms.
Now, the temp path is checked during install to make sure it is writable.

Before, the temp path was not check during install. With some configuration,
FRSS was not working because of a non-writable temp directory. It happened
with XAMPP on MacOS X but it might be the case for other platforms.
Now, the temp path is checked during install to make sure it is writable.

See FreshRSS#3310
@aledeg aledeg added this to the 1.18.0 milestone Jan 1, 2021
@Alkarex
Copy link
Member

Alkarex commented Jan 1, 2021

Great 👍
As a future improvement, maybe we could factorise The HTTP server must have write permissions to avoid having to translate it multiple times.

@Alkarex
Copy link
Member

Alkarex commented Jan 1, 2021

Ah, and as another improvement, it would be nice to have the same check result in the Installation check view
https://github.com/FreshRSS/FreshRSS/blob/master/app/views/update/checkInstall.phtml

But this is already better than the current status :-)

@aledeg
Copy link
Member Author

aledeg commented Jan 1, 2021

As I changed the wording by using a parameter, there is a lot of things to factorize. Not only the HTTP part. Actually, the whole message can be modified to keep only one.

@Alkarex Alkarex merged commit 08d7696 into FreshRSS:master Jan 2, 2021
@aledeg aledeg deleted the enhance/install-check branch January 2, 2021 23:01
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.

Install check that temp path is writeable
2 participants