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

Avoid creating jetpack-temp directory in WP root #19162

Closed
wants to merge 2 commits into from
Closed

Avoid creating jetpack-temp directory in WP root #19162

wants to merge 2 commits into from

Conversation

szepeviktor
Copy link
Contributor

@szepeviktor szepeviktor commented Mar 13, 2021

Fixes #mypain

Changes proposed in this Pull Request:

Try to create jetpack-temp directory in wp-content in the first place.

Some background

I'd like to keep all installations highly clean: git + Composer + separate directory for WordPress core files.

kép
(🖼️ file change alert notification)

So the document root contains 2 directories:

  1. core
  2. and wp-content

It needs 2 lines in wp-config.
https://wordpress.org/support/article/giving-wordpress-its-own-directory/

As of today Jetpack writes into core's directory (ABSPATH).
This PR prioritizes uploads and wp-content directory.

@szepeviktor
Copy link
Contributor Author

Error: Project packages/backup is being changed, but no change file in projects/packages/backup/changelog/ is touched!

I've been pwnd! Could you help me phrase a line for the changelog?

@jeherve jeherve added [Package] Backup [Pri] Low [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Mar 15, 2021
@jeherve
Copy link
Member

jeherve commented Mar 15, 2021

I've been pwnd! Could you help me phrase a line for the changelog?

You should be able to follow the instructions here:
https://github.com/Automattic/jetpack/blob/master/docs/monorepo.md#using-the-jetpack-changelogger

Go to that project and use vendor/bin/changelogger add to add a change file.

@jeherve jeherve requested a review from thingalon March 15, 2021 17:32
@szepeviktor
Copy link
Contributor Author

Jeremy! That is SO HARD!!!

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Mar 15, 2021

@jeherve Done 🍏🍏

@jeherve jeherve added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Mar 15, 2021
@jeherve jeherve requested review from vianasw and removed request for thingalon March 15, 2021 17:48
@jeherve
Copy link
Member

jeherve commented Mar 15, 2021

Done 🍏🍏

Thank you! We'll take a look and review soon.

Internal reference: p1615830698055600-slack-CS8UYNPEE

Copy link
Contributor

@vianasw vianasw left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. While we think that this could work, it would directly affect so many sites and we can't be sure if that it's not going to break anything. We think that an approach using a constant would work better in this case (i.e., if there is an install location constant define use that, otherwise fall back to what we have now).

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Mar 18, 2021
@szepeviktor
Copy link
Contributor Author

All right.

👑

I leave it up to you.

BTW This is the new constant: an immutable configuration.

// Define constant values.
Config::init(
    [
        'version' => '1.0.0',
        'filePath' => __FILE__,
        'baseName' => plugin_basename(__FILE__),
        'slug' => 'plugin-slug',
    ]
);

https://github.com/szepeviktor/small-project/blob/master/src/Config.php

@szepeviktor
Copy link
Contributor Author

@jeherve Friendly ping 🏓

@jeherve
Copy link
Member

jeherve commented Jul 31, 2023

I'm afraid I have no news for you. The discussion above still applies:
#19162 (review)

@szepeviktor
Copy link
Contributor Author

Thank you for your response.

I close this PR.

@github-actions github-actions bot removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Backup [Pri] Low [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants