phpBB Ext Check - Diskussion bezüglich Prozedur und Reports

[Mike-on-Tour]

Da schließe ich mich gerne an, ich nutze dieses wunderbare Tool ja regelmäßig und bin sehr froh, dass es das gibt.

[Kirk]

Auch meinerseits ein Herzlichen Dank, eines der wichtigsten Tools zur EXT Prüfung.

[Talk19zehn]

Hello LukeWCS,

heute erstmalig "durchgelaufen"
phpBB Ext Check ohne Fehlerchen: Alle Häkchen korrekt / Grün.

Auch ich danke dir sowie den Beteiligten für euer unermüdliches Tun, Handeln und stetigen Anpassungen.

LG

[StokerP]

Thank you for this great tool!
There is only one thing I find hard to understand and fix. Thats the VariableAnalysis.
The most common warning for me is: Unused function parameter $event.

An example: 174 | WARNING | Unused function parameter $event.

This is the line: public function viewtopic_page($event): void

From:

Code: Alles auswählen

/**
	 * @param \phpbb\event\data $event
	 * @return void
	 */
	public function viewtopic_page($event): void
	{
		if (empty($this->config['topicstats_active']))
		{
			return;
		}

		$location = (int) $this->config['topicstats_location'];

		if (!$this->topicstats->is_valid_location($location))
		{
			$location = topicstats::BEFORE_FIRST_POST;
		}

		$this->template->assign_vars([
			'TOPICSTATS_ENABLED'	=> true,
			'TOPICSTATS_LOCATION'	=> $location,
			'TOPICSTATS_ALL_PAGES'	=> (bool) $this->config['topicstats_all_pages'],
			'TOPICSTATS_AUTHOR'		=> (bool) $this->config['topicstats_author'],
			'TOPICSTATS_MODAL'		=> (bool) $this->config['topicstats_modal'],
		]);
	}

[Kirk]

Hi
Find:

Code: Alles auswählen

	public function viewtopic_page($event): void

Replace with:

Code: Alles auswählen

	public function viewtopic_page(): void

This: $event is not needed in this function.

[Mike-on-Tour]

If you are not using the $event variable within the function of an event listener you do not need to use it, the listener will work as intended, so a function declaration changed to

Code: Alles auswählen

public function viewtopic_page(): void

will work as well and you will no longer get warnings from VA.

Another thing which regularly will be notified by VA is using an associative array of arrays through which you go with

Code: Alles auswählen

foreach ($array as $key => $subarray)
{
	// Do something with $key but not with $subarray
}

This will give you the warning that $subarray is an unused variable (which is correct, but you need it to define the structure of $array).

The answer to your (unspoken) question would be: Ignore the warnings, they are just a formality because VA cannot find the variable within the code where it should be used.

[LukeWCS]

@Stoker

In addition to what Kirk and Mike-on-Tour wrote:

A PHP function doesn't necessarily have to provide a function parameter for every value passed to it. If you don't need any of the data passed to your function, you don't have to accept that data in your function. In your case, you are only using the PHP event as an execution trigger for your own code, but you do not need the event data itself.

But: Take a look at section 4 in the first post of this thread. There you'll find important information to help you evaluate a report more easily. It doesn't all have to be "green"; there are only two check modules where there absolutely must be no errors:

• phpBB PHP Strict Standard Extensions (PPSSE)
• YAMLcheck (YMLC)

[StokerP]

Thank you guys.
That makes sense and its logical. You just have to know that first

I'll take a look at section 4

[StokerP]

Another question, not about the EC itself, but about the reports.
Are we allowed to include them in extensions when we submit? I think the Validation Team can benefit from them.
And include them in the topics where we present the extension? It doesnt tell if the extension actual does what it should, but it is a proof of quality.

[LukeWCS]

Are we allowed to include them in extensions when we submit?

I don't see any fundamental problem with that. If you create a docs folder and save the EC report there, it shouldn't be an issue. This folder is practically a standard location for including README files or changelogs.

I think the Validation Team can benefit from them.

I doubt that makes sense. For one thing, a validator can't trust anything contained in an uploaded ZIP file, because part of their job is to verify that everything is legitimate. And even if a validator were familiar with EC, they wouldn't trust the report because it was generated in an external environment over which they have no control. Such a report could also have been manipulated.

And include them in the topics where we present the extension? It doesnt tell if the extension actual does what it should, but it is a proof of quality.

I'm also rather skeptical about this, because end users who don't program in PHP themselves and/or have no knowledge of extension development can neither analyze nor evaluate such a report. That's one problem. The next problem is that an Ext Check report may well contain warnings (as we have clarified here), and these could deter some end users because they cannot assess them.