Conversation
Co-Authored-By: Javier Casares <javier@casares.org>
Co-Authored-By: Javier Casares <javier@casares.org>
This will be hanlded in #254 instead.
Co-Authored-By: Javier Casares <javier@casares.org>
Co-Authored-By: Javier Casares <javier@casares.org>
There was a problem hiding this comment.
Looks great! Thank you for doing all this, the clarifying comments etc.
This is the tiniest thing -- Line 154 of readme.md is still a bit awkward. Maybe it could say "This example uses the /home/wptestrunner/ folder..."
On line 156, 166 and 436 are we removing the bash part of ```bash because those commands are linux cli and not bash? (for example bash doesn't support a vim or vi command) -- or is there another reason? Should we put a ```linux cli highlight there instead?
Thanks!
This makes the following improvements: - Ensure each function, and file have a proper short description. - Use third-person singular verbs for function summaries. - Enforces the 80 character limit (120 when indentation is present) for DocBlocks. - Code should be self-documenting. Avoid explaining every step of the code. - Multi-line inline comments should start with `/*` not `/**`. Co-Authored-By: Javier Casares <javier@casares.org>
|
Thanks! I still have a few more changes I'd like to make. When I created this PR, I started by copying every documentation change unrelated to adding support for mult-php/multi-commit from the other pull request and my plan was to make any further changes for consistency, and pose some discussion points within the code here. The README file is still on my to do list. The reason for removing the syntax highlighting for |
|
I see! I vote we leave them in in that case. Should we make a different PR for the readme? Re:
|
kittenkamala
left a comment
There was a problem hiding this comment.
I know this is still in progress, but just noticed these:
| // Define the array of operations to perform, depending on the SSH connection availability. | ||
| // If no SSH connection string is provided, add a local operation to the array. | ||
| // If an SSH connection string is provided, add a remote operation to the array. | ||
| // When am SSH connection string is not provided, add a local operation to the array. |
There was a problem hiding this comment.
Typo:
| // When am SSH connection string is not provided, add a local operation to the array. | |
| // When an SSH connection string is not provided, add a local operation to the array. |
| * Transfer the built WordPress codebase to the remote test environment. | ||
| * | ||
| * When an SSH connection is configured, rsync is used to copy the files | ||
| * required tp rim the WordPress PHPUnit test suite. |
There was a problem hiding this comment.
typo?
| * required tp rim the WordPress PHPUnit test suite. | |
| * required to run the WordPress PHPUnit test suite. |
This splits out the documentation updates from #212 that were unrelated to the overall goals of that pull request.