-
Notifications
You must be signed in to change notification settings - Fork 69
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
Feature/zero config setup #115
Conversation
bin/setup-wp.js
Outdated
dbPass: "wp", | ||
wpUser: "ivan.grginov", | ||
wpPass: "wp-pass", | ||
siteName: "ImeStranice" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a test or? If any default text will be placed it should be in English :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temp stuff, there's really no need to set any defaults here, will remove. 👍
wp-info.json
Outdated
"db-name": "infinum_boilerplate_dev", | ||
"db-user": "wp", | ||
"db-pass": "wp", | ||
"wp-user": "ivan.grginov", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with having your name, but it would be safer to use something totally generic, like infinum.user
Changes in last push
To Do
|
README.md
Outdated
@@ -12,6 +12,8 @@ This repository contains all the tools you need to start building a modern WordP | |||
|
|||
First you need to install WordPress locally, using any of the local development environment you prefer. You can use XAMPP, MAMP, WAMP, VVV, Docker or Laravel Valet. You'll also need to have [Node.js](https://nodejs.org/en/), [Composer](https://getcomposer.org/) and [WP-CLI](https://wp-cli.org/) installed. | |||
|
|||
**Note regarding `Windows` - If you're installing this on `Windows` and you're using `XAMPP`, use `XAMPP` shell (Control Panel -> Shell) rather than `cmd` ** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't rendered as bold. A better approach imo would be to add this as a subheading like:
Windows installation
If you're installing this on Windows, and you are using XAMPP
, use XAMPP
shell (Control Panel -> Shell) rather than the Windows command line prompt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this a bit so it no longer needs to be bolded.
@@ -66,18 +68,18 @@ Once you're done with the above setup (no matter which method you used), run `np | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the top where the ## Initial Setup
is, we should add a note that if you are running from the vagrant ssh, these are all installed (node, composer and wp-cli)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for the manual part, besides running npm start
, shouldn't composer install
be ran as well? Line 41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You still need wp-cli
for vagrant (since wp core download
is run outside vagrant). And you don't actually need composer regardless (at least not for installation) since we're running it using npx
.
As for composer install, that is done just after renaming (actually it's update
not install
). (1. npm install
, 2. renaming, 3. npx composer update
, 4. wp core donwload
)
readme.txt
Outdated
@@ -0,0 +1,106 @@ | |||
=== Infinum WordPress Boilerplate === | |||
Contributors: mustra, dingo_bastard, ivangrginov | |||
Tags: Starter Theme, Webpack, Node, NodeJS, npm, npx, Build Process, Autoprefixr, SCSS, SASS, LESS, Babel, Browsersync, Accessibility Ready, Custom Colors, Custom Header, Custom Logo, Custom Menu, Editor Style, Featured Images, Flexible Header, Footer Widgets, One Column, Theme Options, Translation Ready, Two Columns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be very careful when adding Accessibility Ready
tag. As any theme submitted with this tag has to go through a special review on wordpress.org.
Also, there seem to be some nonstandard tags here, which could trigger some reviewers:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Removed most of the tags since we don't support pretty much anything in the backend without coding it yourself :)
readme.txt
Outdated
|
||
= Requirements = | ||
|
||
* PHP 7 or greater (recommended: PHP 7 or greater) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(recommended: PHP 7.2 or greater)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
OMFG the readme! ❤️ ❤️ 😄 I'd only change one thing: the aliasing for the phpcs. We should add to "scripts" : {
"check-cs": "@php ./vendor/bin/phpcs --extensions=php --ignore=/vendor/",
"fix-cs": "@php ./vendor/bin/phpcbf --extensions=php --ignore=/vendor/",
} That way you can just run (from where the composer is setup for development) composer check-cs . Or specify the directory, and you'll have linting that's fully working. Aliasing can cause some issues, especially if you have globally installed Other than that really awesome work on the readme, looks sick 😄 👍 |
Tested it and seems to work great, I'll all for it! Added the scripts and changed the readme to reflect the changes. Regarding the |
Fix override globals issue.
…lerplate into feature/zero-config-setup
Still a WIP, opening a PR so this is all tracked.
Updated setup process for boilerplate that should be as simple and intuitive as possible while working on all platforms.
Currently it's 1 script (
npm run setup
) for setting everything up, after which user can follow the manual WordPress wizard install (by going to website's local url) or try the automatic setup depending on his local dev setup.Would appreciate any feedback on intuitiveness of this.
Currently done
rename.js
now works no matter how many times you run it (saves old state totheme-manifest.json
)theme-manifest.json
with initial values we replace usingrename.js
To do
composer
usingnpx
)