πŸ‡§πŸ‡ΎBelarus @pavel.bulat

Account created on 29 March 2018, over 6 years ago
  • Senior Full-Stack Developer at SystemSeedΒ  …
#

Merge Requests

Recent comments

πŸ‡§πŸ‡ΎBelarus pavel.bulat

Thanks @beloglazov91 , code changes look good.

πŸ‡§πŸ‡ΎBelarus pavel.bulat

The patch had an issue:

Error: Call to undefined method Drupal\Core\Field\BaseFieldDefinition::id() in limited_field_widgets_field_widget_third_party_settings_form() when you try to open the `Manage Form Display` tab with fields that were created on a code level.
For example: a Commerce Order has an Unlimited Order Items field. The field definition for the Order Items doesn't have id().

Steps to reproduce

1) Install Drupal Commerce and Commerce Order
2) Create a new order type
3) Open Manage Form Display tab

In the last commit, I added a check if ($field_definition instanceof FieldConfigInterface). It fixed the issue, but I am not sure that it was the right way to fix it, so feel free to review it.

πŸ‡§πŸ‡ΎBelarus pavel.bulat

I have realized that the issue is related to the https://www.drupal.org/project/limited_field_widgets/issues/3201950 ✨ Implement custom validation constraint Needs review since the issue was introduced by patch #5 in the https://www.drupal.org/project/limited_field_widgets/issues/3201950 ✨ Implement custom validation constraint Needs review

πŸ‡§πŸ‡ΎBelarus pavel.bulat

Hi @goz, @benjifisher.

Again, I think we should agree on the order of parameters. (See Comment #2.)

Maybe let's use "$metrics" array as a parameter instead?

   $expected_metrics = [
      'stylesheet_count' => 2,
      'script_count' => 1,
      'stylesheet_bytes' => 40150,
      'script_bytes' => 7250,
      'query_count' => 0,
      'cache_get_count' => 1,
      'cache_set_count' => 0,
      'cache_delete_count' => 0,
      'cache_tag_checksum_count' => 0,
      'cache_tag_is_valid_count' => 1,
      'cache_tag_invalidation_count' => 0,
    ];
    $this->assertMetrics($performance_data, $expected_metrics);

I assume the amount of metrics will constantly grow, so using them as parameters won't be optimal. Also, I think it is more readable since you can immediately understand which number to change in case of failed test.

Just summarising the options that we have:

  1. parameters
    cons:
    - the amount of them may grow
    - harder to read numbers if name of each parameter is not highlighted by IDE on the test level)
    - you need to pass all previous parameters to the assertMetrics to be able to check the last one if needed. When a new important metric is introduced, it will be hard to re-order parameters, since a lot of tests will be already using the assertMetrics, so, in this case, we always would need to add all previous parameters.
    pros:
    - each parameter has a type
    - could be highlighted by IDE
  2. array
    cons:
    - looks like it also will be copy pasted from other tests, since it requires a bit more effort to be created. Potentially we can add an example of the array in the description for the assertMetrics helper)
    - doesn't have types
    pros:
    - more readable on the test level, so easy to maintain
    - doesn't have any problems when new metrics are added

I will be happy to hear your thoughts. Thanks in advance.

πŸ‡§πŸ‡ΎBelarus pavel.bulat

Thanks for the update @farhadhf , I have tested it and it works as expected.

πŸ‡§πŸ‡ΎBelarus pavel.bulat

Hi @sboden , I have tested it with

"name": "drupal/config_split",
"version": "2.0.0-rc4",

"name": "drupal/config_filter",
"version": "2.4.0",

"name": "drupal/config_ignore",
"version": "2.4.0",

and I can confirm that all ignored configs will be exported during `drush cex`. I think it should work this way. The most important is that ignored configs won't be imported during `drush cim`. I have tested it and ignored configs weren't imported in my case.

Production build 0.71.5 2024