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

phpDocs - various updates #848

Merged
merged 20 commits into from
Aug 10, 2016
Merged

phpDocs - various updates #848

merged 20 commits into from
Aug 10, 2016

Conversation

ramiy
Copy link
Contributor

@ramiy ramiy commented Aug 8, 2016

No description provided.

@DevinWalker
Copy link
Member

DevinWalker commented Aug 9, 2016

Hey @ramiy we just got some time to review this and

  1. Please ensure periods are not removed.
  2. We can use @Package instead of @subpackage , because it is going to deprecate soon https://phpdoc.org/docs/latest/references/phpdoc/tags/subpackage.html
  3. Ensure all comments are closed properly - @ravinderk said he saw a place where the comment wasn't closed by the link he provided didn't have the line number so I can't reference it. He'll post when he comes back online.

@ramiy
Copy link
Contributor Author

ramiy commented Aug 9, 2016

@DevinWalker We already use @Package, Do you want me to delete all the @subpackage?

@DevinWalker
Copy link
Member

If you could swap out the usage of @subpackage to @packagethat'd be great.

@ramiy
Copy link
Contributor Author

ramiy commented Aug 9, 2016

Replace this:

 * @package     Give
 * @subpackage  Classes/Give_Cron

with:

 * @package     Give
 * @package     Classes/Give_Cron

or:

 * @package     Classes/Give_Cron

or:

 * @package     Give_Cron

@ramiy
Copy link
Contributor Author

ramiy commented Aug 9, 2016

By the way, WordPress core still uses @subpackage, they did not deprecated them.

@DevinWalker
Copy link
Member

@ramiy - let's go with * @package Classes/Give_Cron

@DevinWalker DevinWalker merged commit b4cfbbe into impress-org:release/1.6 Aug 10, 2016
DevinWalker pushed a commit that referenced this pull request Aug 10, 2016
…into ravinderk-hotfix/issues/722

Resolved conflict between PR #848 and #836
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.

None yet

2 participants