Skip to content

Conversation

@GrahamCampbell
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove this blank line and not, for example, in the diff above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other's are blank lines of comment. This is just a dodgie blank line with nothing on it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to send a pull request later to standardise the annotation separation.

I want to keep the PRs atomic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add the missing * instead IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof Should I fix all cases then? There's loads of them...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I would fix this dodgie blank line in the expected way.

Then, fixing the separation of params and return values could be done in a separate PR to make it easier for maintainers to review it, or it could be included in the same PR to have everything fixed at once. I don't know what @pilot prefers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @pilot. What would you like me to do here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell make all in one PR, just to fix all at once pls

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do.

@GrahamCampbell GrahamCampbell changed the title Fixed a few docblocks Fixed a load of docblocks Feb 28, 2015
@GrahamCampbell
Copy link
Contributor Author

Ping @pilot, @stof. Done. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove dot at the end for all places, looks weird

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is happening because you're miss using the short description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, a dot is required because this is a sentence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell: in this case, you just need to remove the colon before the period.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I'll do a find and replace for all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@GrahamCampbell
Copy link
Contributor Author

Ping @pilot.

pilot added a commit that referenced this pull request Mar 13, 2015
Fixed a load of docblocks
@pilot pilot merged commit 2d33d76 into KnpLabs:master Mar 13, 2015
@GrahamCampbell GrahamCampbell deleted the doc branch March 13, 2015 21:49
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.

5 participants