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

Remove WP installers with profile.d #3

Open
wants to merge 2 commits into
base: frc
Choose a base branch
from
Open

Conversation

okko
Copy link

@okko okko commented Oct 16, 2016

No description provided.

@josepfrantic josepfrantic force-pushed the frc branch 2 times, most recently from 0293467 to 9757004 Compare November 10, 2016 08:55
@pnu
Copy link
Member

pnu commented Nov 15, 2016

Seems like an good idea, but have you tested does this cause any other issues, in normal operations? Removing files seems a bit brutal; how nicely does WP fallback from that? Are there any sites where this has been tested?

@okko
Copy link
Author

okko commented Nov 15, 2016

All of these should fail within Heroku anyway because a) they can't write to disk b) even if they could, it would be reverted when dyno restarts. Has been tested and running on a production site for 30 days without any issues.

@JanneAalto
Copy link

JanneAalto commented Nov 15, 2016

We talked with Lauri about this, and there might be a problem when wp database update is needed when wp is updated.

I have to look into it a bit more

@@ -0,0 +1,5 @@
rm -rf $HOME/web/wp/wp-admin/install.php

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not going to work in wp-bedrock-seed projects.

$HOME is /app in heroku but the actual filepath would be ./bedrock-on-heroku/web/wp/wp-admin/install.php

Since this is a bash script anyway, we could check if the directory "bedrock-on-heroku" exists, in order to know the right location to remove those files.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or rm -rf both locations, other will fail silently?

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.

4 participants