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

added draft test files #132

Merged
merged 2 commits into from
Oct 2, 2014
Merged

added draft test files #132

merged 2 commits into from
Oct 2, 2014

Conversation

lucatume
Copy link
Contributor

I've added a draft for test files to be used in place of real source code. Open to input for more/better job to be done.

@JDGrimes
Copy link
Contributor

That looks pretty thorough, as far as the docs are concerned. One thing that I would like to see is consistency with your indentation of the docblocks. There should always be a space before the *.

So like this:

/**
 * Comment.
 */

Not this:

/**
* Comment.
*/

Something that could be added is function/method/hook usage in a file/function/method (see #85).

Maybe I'm picky, but in the other file names we use dashes and not underscores, and I think we should be consistent with that.

@lucatume
Copy link
Contributor Author

Ok, I will modify the files accordingly.

Luca

On 29 Sep 2014, at 16:01, J.D. Grimes [email protected] wrote:

That looks pretty thorough, as far as the docs are concerned. One thing that I would like to see is consistency with your indentation of the docblocks. There should always be a space before the *.

So like this:

/**

  • Comment.
    */
    Not this:

/**

Maybe I'm picky, but in the other file names we use dashes and not underscores, and I think we should be consistent with that.


Reply to this email directly or view it on GitHub.

@lucatume
Copy link
Contributor Author

lucatume commented Oct 1, 2014

I've modified comment indentation, IDE did actually, and renamed the files, ok?
Will tackle issue as soon as this one goes.

@JDGrimes
Copy link
Contributor

JDGrimes commented Oct 1, 2014

That looks good to me.

@lucatume
Copy link
Contributor Author

lucatume commented Oct 1, 2014

ok,

will look into another issue then

On Wed, Oct 1, 2014 at 2:55 PM, J.D. Grimes [email protected]
wrote:

That looks good to me.


Reply to this email directly or view it on GitHub
#132 (comment).

Luca

Rarst added a commit that referenced this pull request Oct 2, 2014
@Rarst Rarst merged commit 041b08f into WordPress:master Oct 2, 2014
@Rarst
Copy link
Contributor

Rarst commented Oct 2, 2014

Thank you very much! Pleasure working with you on contributor day! :)

@Rarst Rarst mentioned this pull request Oct 2, 2014
@lucatume
Copy link
Contributor Author

lucatume commented Oct 2, 2014

thanks, it was my pleasure and plan on doing it more frequently

On Thu, Oct 2, 2014 at 10:44 AM, Andrey Savchenko [email protected]
wrote:

Thank you very much! Pleasure working with you on contributor day! :)


Reply to this email directly or view it on GitHub
#132 (comment).

Luca

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.

3 participants