Improvements (chapters 1 to 12) #16
Conversation
Add a bunch of full stops to comments and task descriptions. Rephrase sentences to adapt their appearance (e.g., 'Shows all...' -> 'Show all...'). Remove trailing line-feeds as only a few files did have one.
Add a bunch of full stops to comments and task descriptions. Rephrase sentences to adapt their appearance (e.g., 'Shows all...' -> 'Show all...'). Remove trailing line-feeds as only a few files did have one.
Add a bunch of full stops to comments and task descriptions. Replace 'unused variable' with 'unreachable statement'.
Add a bunch of full stops to comments and task descriptions. Use the same format for all @param comments. Remove trailing line-feeds as only a few files did have one.
Add a bunch of full stops to comments and task descriptions. Remove trailing line-feeds as only a few files did have one. Don't use require_once as function, it is a language construct.
There was a problem hiding this comment.
I think these changes are incorrect; according to the docs, type should be before the name of the param; http://www.phpdoc.org/docs/latest/references/phpdoc/tags/param.html
There was a problem hiding this comment.
I aggree, and I always write @param PHPDoc comments à la type, variable name, description. However, all other comments throughout the workshop materials are in the manner stated in my commit.
There was a problem hiding this comment.
Perhaps it's best to wait for @maartenba to respond.
If you intend to add the missing newlines to all files, I think it's easier to do so with some simple scripting, like sed (for an example, see: http://unix.stackexchange.com/questions/31947/how-to-add-a-newline-to-the-end-of-a-file).
There was a problem hiding this comment.
(Oops, clicked the wrong "add a line note" button.. should've clicked the one below)
|
Good changes so far! |
|
Hi Maarten, I was just about to change what I already discussed with @thaJeztah—newlines and the end of each file, correct order of What do you think about this? |
|
Sounds like a plan, I like the idea |
tfrommen
changed the title
|
Lot better, thanks! Two remarks;
|
Remove Thumb.db file. Improve shell script and batch file - or was there a reason for actually using the second (!) parameter?
|
@thaJeztah The I did not see any binary files using PhpStorm's built-in git features. I just reverted any changes to images as well as the icon file. Thanks for that. |
tfrommen
changed the title
|
@tfrommen thanks; weird though, if I view the files in your latest commit, GitHub won't show the images: Viewing the original commit, GitHub also shows them correctly; https://github.com/tfrommen/phpstorm-workshop/commit/82a3b9c0cabb79ff0bb0173bd036cd43191b31f4#diff-787d37fecfdeb310bccbbf2bc75487f2 So, not really sure what's wrong there.. weird |
|
Looks good to me, shall I merge it in? |
|
@maartenba it looks good, but if you could check the PNGs just to be sure nothing's wrong with them (to confirm it's just a GitHub quirk). |
Super awesome contribution. Thanks both!
|
Thanks @tfrommen and bedankt, Maarten :) |
|
Thanks to both of you! |
3 participants
Besides minimal changes regarding a few
@paramPHPDoc comments in order to use the same format, this pull request contains several improvements for the reguloar twelve chapters:require_onceandincludeas functions, they are language constructs.