πŸ‡³πŸ‡±Netherlands @ttadegraaff

Account created on 17 October 2010, over 13 years ago
#

Recent comments

πŸ‡³πŸ‡±Netherlands ttadegraaff

thomasdegraaff β†’ made their first commit to this issue’s fork.

πŸ‡³πŸ‡±Netherlands ttadegraaff

Looking at the code it seems that the dependency injection for the entity type manager service is removed, and the existing dependency injection for the logger service and the radioactivity reference updater service is not used anymore.

Dependency injection is the preferred method for accessing and using services in Drupal 8 and should be used whenever possible. Is there a reason not to use dependency injection in this case?

πŸ‡³πŸ‡±Netherlands ttadegraaff

I can't get the test to pass, and fail to understand what goes wrong. It's quite hard to debug because there's no error messages showing what goes wrong. Maybe someone more experienced in writing functional tests can help?

πŸ‡³πŸ‡±Netherlands ttadegraaff

Patch adds default incident process ratio setting of 1.

πŸ‡³πŸ‡±Netherlands ttadegraaff

@Sutharsan Thx for the feedback.

To answer your questions:

1. I'll use an example to explain it. Let's suppose an incident emits 10 energy. Let's suppose there have been 10 incidents. When the incident process ratio is 1, then these 10 incidents will emit 100 energy in total.

Now the incident process ratio is set to 0.5.

There have been 10 incidents again. Now statistically half of the 10 incidents will be processed, so that is 5 incidents. When each incident then emits 10 / 0.5 = 20 energy, then statistically the total energy emitted will be 5 x 20 = 100 energy in total.

This way statistically 10 incidents will store the same amount of energy notwithstanding the incident process ratio that is set.

The reason I explain this in the readme is to prevent users to change the emitted energy to do this correction themselves. Like as when someone sets the incident process ratio from 1 to 0.1 then this person might change the setting for emitted energy from 10 to 1 to not mess up the stats.

2. Thanks for the explanation of the rationale of not concatenating translated strings. I'll fix these issues.

3. I had an issue when testing the incident process ratio functionality that the actual process ratio would not change for a test page after changing the incident process ratio setting. Only after clearing the cache reloading the page would trigger incidents with the configured process ratio setting. The incident process ratio is passed to the javascript via drupalSettings and this seems to be cached so clearing the cache when the setting is changed seemed the right way to fix this.

4. I'll fix this.

πŸ‡³πŸ‡±Netherlands ttadegraaff

Fixes broken link to admin settings page in previous patch, and removes debug console.log in javascript.

πŸ‡³πŸ‡±Netherlands ttadegraaff

Thanks for the feedback @Stuharsan. I'll fix the issues you mentioned.

The setting should be site wide indeed. And the emitted value should be divided by the incident process ratio so the values submitted with different incident process ratios are comparable. I'll provide a patch later on.

πŸ‡³πŸ‡±Netherlands ttadegraaff

Patch attached.

A 'Incident process ratio' setting has been added to the radioactivity field formatter. This setting is a number between 0 and 1 and defaults to 1. If it is set to 0.2 then one in the five incidents is posted to the backend. To accomplish this the incident process ratio is passed to the triggers.js where it is used to determine if a incident should be posted to the backend or not.

Production build 0.69.0 2024