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

Update Readme.md with theme instructions. #51

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

Conversation

StevenDufresne
Copy link
Contributor

As mentioned in #50, it isn't clear what should be done with the test-theme folder.

I think it's easier if we explicitly instruct users to pass in the path for now.

Fixes #50.

@carolinan
Copy link
Contributor

I need to test if this actually work on Windows...

@carolinan
Copy link
Contributor

I may be doing something wrong.

If the path to theme review action is D:/theme-review-action/
and my theme is in: D:\wordpress-develop\src\wp-content\themes\twentytwentyone

Then npm run start --pathToTheme=../wordpress-develop\src\wp-content\themes\twentytwentyone
nor he command with the double -- works.

-I think that -- duplication might have been a typo?

The error on Windows is still present:

× Copying theme files into the environment...
Error: Cannot copy '.' to a subdirectory of itself, 'D:\theme-review-action\test-theme'.

@StevenDufresne
Copy link
Contributor Author

I may be doing something wrong.

If the path to theme review action is D:/theme-review-action/
and my theme is in: D:\wordpress-develop\src\wp-content\themes\twentytwentyone

Then npm run start --pathToTheme=../wordpress-develop\src\wp-content\themes\twentytwentyone
nor he command with the double -- works.

-I think that -- duplication might have been a typo?

The error on Windows is still present:

× Copying theme files into the environment...
Error: Cannot copy '.' to a subdirectory of itself, 'D:\theme-review-action\test-theme'.

The extra -- is necessary, it tells npm to pass along the arguments, otherwise it doesn't.

Try:
npm run start -- --pathToTheme=..\wordpress-develop\src\wp-content\themes\twentytwentyone

@carolinan
Copy link
Contributor

I did already try with the extra --, the result is the same.

Log from today:

PS D:\theme-review-action> npm run start -- --pathToTheme=..\wordpress-develop\src\wp-content\themes\twentytwentyone

> [email protected] start D:\theme-review-action
> node bin/program.js

Please ensure docker is running.

Test Version: 1.4.11
Testing Ports: 8484/8485

Steps:
× Copying theme files into the environment...
Error: Cannot copy '.' to a subdirectory of itself, 'D:\theme-review-action\test-theme'.
× Checking theme's basic structure
Error: Command failed with exit code 1: npm run check:structure
D:\theme-review-action\actions\structure-check\index.js:86
                throw Error('Failed basic structure.');
                ^

Error: Failed basic structure.
    at D:\theme-review-action\actions\structure-check\index.js:86:9
    at Object.<anonymous> (D:\theme-review-action\actions\structure-check\index.js:90:3)
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
    at Module.load (internal/modules/cjs/loader.js:933:32)
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] start: `node index.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Carolina\AppData\Roaming\npm-cache\_logs\2021-07-13T03_21_40_475Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] check:structure: `cd ./actions/structure-check && npm run start`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] check:structure script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Carolina\AppData\Roaming\npm-cache\_logs\2021-07-13T03_21_40_605Z-debug.log

> [email protected] check:structure D:\theme-review-action
> cd ./actions/structure-check && npm run start


> [email protected] start D:\theme-review-action\actions\structure-check
> node index.js

Running structure check.

Structure Check Errors:

The theme is required to have an index.php file.

The theme is required to have a style.css file.

The theme is required to have a screenshot.png or screenshot.jpg file.

@StevenDufresne
Copy link
Contributor Author

StevenDufresne commented Jul 16, 2021

I don't have the problem on my windows VM in both bash or powershell:

Can you make sure of the following:

  1. You have the newest theme-review-action changes.
  2. You have an updated version of npm.
  3. You have an updated version of node.

@StevenDufresne
Copy link
Contributor Author

I'm not sure if we need this to work on your machine seeing that the npm run {command} -- --prop API is well documented and appears specific to your environment.

@carolinan
Copy link
Contributor

carolinan commented Jul 16, 2021

Except I am not the only person experiencing these errors. I had to recommend another tester to place the theme in the folder manually.

What are the minimum and or recommended versions of npm and node?
I have not updated to npm 7 because as far as I know Gutenberg still does not work with 7. (And I still need the build to work for Gutenberg :) )

@StevenDufresne
Copy link
Contributor Author

Except I am not the only person experiencing these errors. I had to recommend another tester to place the theme in the folder manually.

Yep, definitely agree with that but this PR itself is not platform-specific. The problems you are experiencing are part of the wider windows issue mentioned in #18.

What are the minimum and or recommended versions of npm and node?
I'm not sure that we have that figured out yet. Can you share your npm and node versions?

npm -v
node -v

@carolinan
Copy link
Contributor

NPM 6.14.13
node v14.17.3

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.

Feedback on readme instructions
2 participants