Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Replace ReturnsTag with DisallowedDocTags sniff #8

Merged
merged 1 commit into from
Oct 10, 2017

Conversation

thiemowmde
Copy link
Collaborator

@thiemowmde thiemowmde commented Aug 16, 2017

This is split from #7, and will make a minor subset of the changes made in #7 obsolete. I will rebase #7 when this is merged.

I did run this new sniff on the Wikibase.git code base, and only found about a dozen violations, all of them intended (mostly @licence).

Note that this branch does not run the PHPCBF test. This will change with #7. This is intentional. I did not wanted to copy to much from #7, to not make this patch to complicated.

class Wikibase_Sniffs_Commenting_DisallowedDocTagsSniff implements PHP_CodeSniffer_Sniff {

/**
* @var array Set a value to null if a tag is forbidden, but no automatic fix should be made.
Copy link
Member

Choose a reason for hiding this comment

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

This needs to document which of key and value is the incorrect and which is the correct value, precisely because of the confusing inconsistency which makes it hard to infer this from the array value :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What are you missing? The pretty much only way to see this comment is when you look at the code. The defaults are there, directly below. They can be overridden via <properties>.

All that should be better documented, I agree. But this is for a later patch.

Copy link
Member

Choose a reason for hiding this comment

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

The defaults are there, directly below.

Yes, and looking at the first defaults, I have no idea if @cover is a misspelling of @covers or @cover is the correct version of the misspelling @covers :) but I guess it’s more clear with the following entries, and allowing the null value only makes sense for one of those interpretations.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Again, I'm not really sure what your question is. I made the list you can see here in the code based on tons of experience with this stuff, based on actual statistics I made with all our code bases (including core), and based on what PHPDoc.org as well as Doxygen describe. Bad tags left, good tags right.

As said, I will document this later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the documentation. Hope this helps.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, much better IMO :) immediately understandable now.

wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-Wikibase that referenced this pull request Aug 17, 2017
A tag named "@ticket" does not exist anywhere else.

I found these while working on
wmde/WikibaseCodeSniffer#8

Change-Id: I7519154e0ae3be5dcb68e15ed604de4a95354b02
@lucaswerkmeister lucaswerkmeister merged commit e54b9a2 into master Oct 10, 2017
@lucaswerkmeister lucaswerkmeister deleted the disallowedDocTags branch October 10, 2017 09:25
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-WikibaseQualityConstraints that referenced this pull request Oct 11, 2017
Found and fixed with wmde/WikibaseCodeSniffer#8 [1].

[1]: wmde/WikibaseCodeSniffer#8

Change-Id: I855fca0b4279f351802d05a6520cde747a3c13d0
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-Wikibase that referenced this pull request Oct 11, 2017
This will soon be checked by our code sniffer, see
wmde/WikibaseCodeSniffer#8

Change-Id: I4fe0aa4e5730d151919d3106c8ea75075118e42a
wmfgerrit pushed a commit to wikimedia/data-values-value-view that referenced this pull request Oct 12, 2017
This is the canonical form, see
wmde/WikibaseCodeSniffer#8

Change-Id: Iae69c5818fe33698c794b95d1b3e122115e2f208
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants