Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

Feature: Require @return & Require @since #27

Merged
merged 16 commits into from
Dec 5, 2017
Merged

Conversation

aubreypwd
Copy link
Contributor

@aubreypwd aubreypwd commented Aug 24, 2017

A new approach from #4 which just focuses on @return and @since and it's requirements from scratch. This needs a lot of testing, but seems to be working well in Sublime and Atom. Please load up some of your recent projects and see if @return is working as you expect.

Closes #8 and #6

This also sets up a basic structure for adding new sniffs.

@aubreypwd aubreypwd self-assigned this Aug 24, 2017
@aubreypwd aubreypwd added this to the 1.1 milestone Aug 24, 2017
@aubreypwd aubreypwd changed the base branch from master to release-1.1 August 24, 2017 15:49
@aubreypwd aubreypwd requested a review from JayWood August 24, 2017 16:02
@aubreypwd aubreypwd changed the title Feature/@return Feature: Require @return & Require @since Aug 24, 2017
@aubreypwd
Copy link
Contributor Author

@JayWood Just putting a request out there to test this in storm and get your blessing.

@aubreypwd
Copy link
Contributor Author

Just added the Discussion Needed label because I think we need to talk to the FED's about this. They don't really use @since but I (and others) think they should too. Going to wait until FED/BED call to discuss (mostly that it's coming, unless someone vetos it).

@aubreypwd
Copy link
Contributor Author

	/**
	 * Get single.
	 *
	 * Need to implement in sub-class.
	 *
	 * @since Unknown
	 *
	 * @param array $insert_args Insert arguments.
	 *
	 * @todo docblock needs-documentation
	 */
	abstract function get_single( $insert_args = array() )

Noting that on this, I'm getting a nag to add @return?

@jrfoell
Copy link
Contributor

jrfoell commented Oct 20, 2017

👍

I approve this change and would actually argue that requiring @return on the abstract docblock is a feature not a bug. In this case since the function can have no body, the documentation is responsible for letting the implementer know what the function should return (array, string, int, mixed, etc.). In the case when it doesn't return anything - we should specify void. I realize this goes against some of our other established @return use cases, but I think it makes sense here.

@aubreypwd
Copy link
Contributor Author

Yeah, but it will require an @return even if you haven't got a return in the abstracted method (for obvious reasons). It's still an accident. I agree the return should be documented, but if the extended methods don't return anything, that would end up being false.

@aubreypwd aubreypwd added 2 Votes and removed 1 Votes labels Oct 24, 2017
@aubreypwd aubreypwd self-assigned this Oct 24, 2017
@aubreypwd aubreypwd merged commit 2c81833 into release-1.1 Dec 5, 2017
@aubreypwd aubreypwd deleted the feature/@return branch December 5, 2017 21:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants