Ensure that various directories are writeable

[naresh-kumar-babu]

Partial Fix #388 .

[yaronkoren]

This looks good - although I would change the commit message, because this patch does not add the function make_dir_writable() (the previous patch already did that).

[naresh-kumar-babu]

This looks good - although I would change the commit message, because this patch does not add the function make_dir_writable() (the previous patch already did that).

Updated.

[yaronkoren]

Sorry for the delay. I have two comments:

• This is pretty minor, but I think a better wording in the commit summary would "Ensure that various directories are writeable...", rather than "Make various directories writeable...", because most of the time make_dir_writable() does nothing. (Maybe the function should have a better name, given that, but that's another story.)
• I think I never noticed before that the Wikiteq code contains that tiny update-images-permissions.sh script. It seems like a silly script - it basically just calls one line of code. Even if you add in the print command and that (maybe pointless?) sleep command, that's still just line three lines of code. I think it would be better to put this code directly into run-all.sh.

[naresh-kumar-babu]

Sorry for the delay. I have two comments:

• This is pretty minor, but I think a better wording in the commit summary would "Ensure that various directories are writeable...", rather than "Make various directories writeable...", because most of the time make_dir_writable() does nothing. (Maybe the function should have a better name, given that, but that's another story.)
• I think I never noticed before that the Wikiteq code contains that tiny update-images-permissions.sh script. It seems like a silly script - it basically just calls one line of code. Even if you add in the print command and that (maybe pointless?) sleep command, that's still just line three lines of code. I think it would be better to put this code directly into run-all.sh.

Done

[yaronkoren]

This looks better, but see my comments.

[naresh-kumar-babu]

This looks better, but see my comments.

Updated.

[yaronkoren]

@naresh-kumar-babu - sorry, I have to apologize! I realize now why they created that separate update-images-permissions.sh file, even though it's so small - it's so that the rest of the setup process can still happen while make_dir_writable() runs across all the updated files/images. (If the code is in a separate script, it will run concurrently/asynchronously, instead of having to finish before the rest of the code runs.) There is a comment that explains this, although I think it's not that clear.

Please restore the code to back to what is was - sorry about the extra work. Although if you could make the comment clearer, that would be helpful. Also, a minor point I realized later: two of their print commands misspell "MediaWiki" as "Mediawiki".

[naresh-kumar-babu]

@naresh-kumar-babu - sorry, I have to apologize! I realize now why they created that separate update-images-permissions.sh file, even though it's so small - it's so that the rest of the setup process can still happen while make_dir_writable() runs across all the updated files/images. (If the code is in a separate script, it will run concurrently/asynchronously, instead of having to finish before the rest of the code runs.) There is a comment that explains this, although I think it's not that clear.

Please restore the code to back to what is was - sorry about the extra work. Although if you could make the comment clearer, that would be helpful. Also, a minor point I realized later: two of their print commands misspell "MediaWiki" as "Mediawiki".

Updated.

[yaronkoren]

Thank you, this looks good. I think you didn't change the comment, but that's probably fine - hopefully what is going on will be understandable.