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

Warn if running plugin from source #2962

Merged
merged 8 commits into from
Jul 9, 2020
Merged

Conversation

swissspidy
Copy link
Collaborator

Summary

Warn users if they forgot to run the build commands.

Helpful if someone just downloads the current master branch as a ZIP file and expects the plugin to "just work".

Relevant Technical Choices

Uses an anonymous function because this cannot really be covered by unit tests and this shouldn't be disabled by others.

To-do

User-facing changes

Screenshot 2020-07-03 at 13 23 19

Testing Instructions

Download the master branch and try to install the plugin on a fresh install, or simply remove the vendor folder to force the admin notice from appearing.


Fixes #1234
See #2961

@swissspidy swissspidy added Type: Enhancement New feature or improvement of an existing feature Group: WordPress Changes related to WordPress or Gutenberg integration Pod: WP & Infra labels Jul 3, 2020
@swissspidy swissspidy requested a review from spacedmonkey as a code owner July 3, 2020 11:29
@codecov
Copy link

codecov bot commented Jul 3, 2020

Codecov Report

Merging #2962 into master will increase coverage by 0.80%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2962      +/-   ##
==========================================
+ Coverage   81.22%   82.02%   +0.80%     
==========================================
  Files         714      718       +4     
  Lines       12016    12068      +52     
==========================================
+ Hits         9760     9899     +139     
+ Misses       2256     2169      -87     
Flag Coverage Δ
#karmatests 63.50% <ø> (+3.88%) ⬆️
#unittests 66.58% <ø> (+0.02%) ⬆️
Impacted Files Coverage Δ
.../app/views/editorSettings/googleAnalytics/index.js 25.00% <0.00%> (-25.00%) ⬇️
assets/src/edit-story/app/font/fontProvider.js 60.00% <0.00%> (-2.50%) ⬇️
assets/src/dashboard/app/api/apiProvider.js 100.00% <0.00%> (ø)
assets/src/edit-story/app/media/mediaProvider.js 100.00% <0.00%> (ø)
assets/src/edit-story/app/media/useMediaReducer.js 100.00% <0.00%> (ø)
...ts/src/dashboard/app/views/editorSettings/index.js 0.00% <0.00%> (ø)
...s/src/edit-story/app/media/utils/createResource.js 100.00% <0.00%> (ø)
...c/dashboard/app/views/editorSettings/components.js 57.14% <0.00%> (ø)
...p/media/media3p/useProviderContextValueProvider.js 100.00% <0.00%> (ø)
...c/dashboard/storybookUtils/formattedUsersObject.js
... and 34 more

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2020

Size Change: -1.26 kB (0%)

Total Size: 1 MB

Filename Size Change
assets/js/edit-story.js 458 kB +252 B (0%)
assets/js/stories-dashboard.js 526 kB -1.52 kB (0%)
ℹ️ View Unchanged
Filename Size Change
assets/css/edit-story.css 269 B 0 B
assets/css/stories-dashboard.css 305 B 0 B
assets/css/web-stories-embed-block.css 515 B 0 B
assets/js/web-stories-embed-block.js 15.7 kB 0 B

compressed-size-action

includes/namespace.php Outdated Show resolved Hide resolved
includes/namespace.php Outdated Show resolved Hide resolved
@spacedmonkey
Copy link
Contributor

Did you consider using wp_die? It maybe over kill, but it is something to consider.

@swissspidy swissspidy force-pushed the add/run-from-source-warning branch 3 times, most recently from bbedb19 to d7df835 Compare July 7, 2020 13:48
@swissspidy swissspidy force-pushed the add/run-from-source-warning branch from d7df835 to a744d10 Compare July 7, 2020 13:49
@swissspidy swissspidy requested a review from spacedmonkey July 8, 2020 09:53
@swissspidy
Copy link
Collaborator Author

wp_die is quite drastic - we don't want the site from working :-)

includes/namespace.php Outdated Show resolved Hide resolved
includes/namespace.php Outdated Show resolved Hide resolved
includes/namespace.php Outdated Show resolved Hide resolved
includes/namespace.php Outdated Show resolved Hide resolved
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

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

👍

@spacedmonkey
Copy link
Contributor

@swissspidy This is good to go!

@swissspidy swissspidy merged commit e0b2d9f into master Jul 9, 2020
@swissspidy swissspidy deleted the add/run-from-source-warning branch July 9, 2020 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: WordPress Changes related to WordPress or Gutenberg integration Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn if running plugin from source
3 participants