Account created on 12 September 2018, almost 6 years ago
#

Merge Requests

More

Recent comments

It could use \Drupal\Core\Utility\DeprecationHelper::backwardsCompatibleCall() and selectively use FileSystemInterface::REPLACE or FileExists::Replace, right?

It looks like the blocking issue is stalled (I'm unsure of how to fix the test). In addition, the GitLab CI issue needs to be merged in order for tests to be run.

So maybe we should release 5.1.0 with what is ready, and hope to get the blocking issue in later.

@smulvih You can't do that in the same step if you have CI/CD, because you can't uninstall the module when the code has been removed from the repository.

The test was working earlier. It should be

  public function testNormalizeNewlines() {
    $form_state = $this->prophesize(FormStateInterface::class)->reveal();
    $element = ['#normalize_newlines' => TRUE];
    $input = "some\r\ndifferent\rline\nendings";
    $expected = "some\ndifferent\nline\nendings";
    $this->assertSame($expected, Textarea::valueCallback($element, $input, $form_state));
  }

openid_connect isn't compatible with D11 either, yet.

Should we change it to core_version_requirement: ^9 || ^10 || ^11? Or just review and merge the changes as-is for now?

Or is that the only thing that needs to change for D11 compatibility, then?

@ankitv18 That gets added to the info.yml in the pipeline stage, so I'm not sure that's the reason.

I know a reroll isn't all that's needed, and that's why I left the status Needs Work.

Regarding #11, are there any plans to format values starting with the other recommended characters?

Hey, I noticed that QueueDatabaseFactory was given the new interface, but QueueFactory does not yet implement the new interface. Should I open a new issue, or can that be fixed here?

@batigolix This issue now pertains only to PHPCS, not to the either pipeline stages.

Further work on this issue requires the fixes for PHPCS to be merged to avoid a merge conflict.

Not sure how to fix this:

WARNING: _cspell_unrecognized_words.txt: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/openid_connect_windows_aad-3471251) 
WARNING: _cspell_json.txt: no matching files. Ensure that the artifact path is relative to the working directory (/builds/issue/openid_connect_windows_aad-3471251) 
ERROR: No files to upload                          
Cleaning up project directory and file based variables
00:00
ERROR: Job failed: command terminated with exit code 1

We should use separate issues for each piece of the pipeline (e.g. phpcs vs cspell)

@miksha The class/attribute used should be specific to VDE, not just every message that has data-download-enabled="true", because other modules could have that.

It wasn't broken. If you read the module page, the project utilized issues on GitHub, not on Drupal.org.

However, this issue is outdated, as issues are being migrated to Drupal.org.

PHPStan failing

 ------ ---------------------------------------------------------------------- 
  Line   src/Form/LinkCheckerAdminSettingsForm.php                             
 ------ ---------------------------------------------------------------------- 
  209    \Drupal calls should be avoided in classes, use dependency injection  
         instead                                                               
 ------ ---------------------------------------------------------------------- 
 ------ ---------------------------------------------------------------------- 
  Line   src/LinkCheckerService.php                                            
 ------ ---------------------------------------------------------------------- 
  161    \Drupal calls should be avoided in classes, use dependency injection  
         instead                                                               
  169    \Drupal calls should be avoided in classes, use dependency injection  
         instead                                                               
  170    \Drupal calls should be avoided in classes, use dependency injection  
         instead                                                               
  181    \Drupal calls should be avoided in classes, use dependency injection  
         instead                                                               
 ------ ---------------------------------------------------------------------- 

Shouldn't that extraction work be done in the constructor instead?

Agreed. Just pass the $configuration to the constructor.

Yes. Make sure you download the patch and put it in a project folder, e.g. "./patches/filename.patch". Don't use the URL for a Merge Request patch, as it can change/break over time.

There appears to be an unrelated CKEditor5 test failure. Rerunning.

Ah, that explains why the MR targeted the wrong branch initially. I had selected the wrong version in the dropdown. Thanks. The MR is targeting 11.x as it should.

Fail      Other      phpunit-16.xml       0 Drupal\Tests\ComposerIntegrationTes
    PHPUnit Test failed to complete; Error: PHPUnit 10.5.29 by Sebastian
    Bergmann and contributors.
    Runtime:       PHP 8.3.10
    Configuration: /builds/issue/drupal-3468139/core/phpunit.xml.dist
    ........S...........................F......................       59 / 59
    (100%)
    Time: 00:00.679, Memory: 10.00 MB
    There was 1 failure:
    1) Drupal\Tests\ComposerIntegrationTest::testExpectedScaffoldFiles with
    data set #0 ('.editorconfig', 'assets/scaffold/files/editorconfig',
    '[project-root]')
    Scaffold source and destination files must have the same contents.
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
     '# Drupal editor configuration normalization\n
    -# @see http://editorconfig.org/\n
    +# @see https://editorconfig.org/\n
     \n
     # This is the top-most .editorconfig file; do not search in parent
    directories.\n
     root = true\n
    /builds/issue/drupal-3468139/core/tests/Drupal/Tests/ComposerIntegrationTest.php:204
    FAILURES!
    Tests: 59, Assertions: 420, Failures: 1, Skipped: 1.

Yes, my mistake. I had to click to expand and view the full output.

None of the tests themselves failed... I'm going to rerun the job.

There we go. The initial fork defaulted to 11.0.x instead of 11.x for some reason. MR using the correct fork.

solideogloria changed the visibility of the branch 3468139-editorconfig-https to hidden.

Back to Needs Work because patch didn't apply and tests failing. Also, the changes need to be converted to a merge request so that it uses GitLab CI.

@jkamizato It might be simple, but it's not secure. Credentials should not be stored in plaintext, and that method is going to do so.

Merge requests need to target 11.x.

Please update the merge request instead of uploading new patches. That way the issue can advance.

It appears that the failures for missing schema are due to the fact that this was removed in 11.x. See function views_removed_post_updates()

'views_post_update_remove_default_argument_skip_url' => '11.0.0',

If I remove that from the view, will it break the test for Drupal 10.3?

So, instead of passing 'bad' data to preg_match can this be fixed in views? And further, is it possible to use a Kernel test instead of a Functional test.

The changed code no longer passes "bad" data to preg_match(). And the actual issue that was reported here is tested, and it requires a bootstrapped Drupal. I think it's better to test that the actual, desired functionality is working.

Should a new issue be opened for adding a configuration form for updating this setting? It's not a big need for me, as I can set the config value. But others might not be able to do that.

I converted the patch to a merge request and rerolled against 11.x

Note that from @daffie in #40, a test still needs to be added for BaseFieldDefinition::createFromFieldDefinition (the wrong function was referenced in the comment above)

A new maintainer (Xon) was added for Choices.

I've been added as a maintainer, and plan todo v11.0.0 beta/rc release early next week and will be closing out issues as they are fixed.

https://github.com/Choices-js/Choices/issues/1150#issuecomment-2270158764

@catch It appears that messages that I was already adding via Ajax are rendered/themed properly without patching BigPipe. But the messages that are added using $this->messenger()->addMessage() (e.g. in a Block's build() function) are not.

namespace Drupal\theme_helper\Plugin\Block;

use Drupal\Core\Block\BlockBase;

/**
 * A theme test block.
 *
 * @Block(
 *   id = "theme_test",
 *   admin_label = @Translation("Theme Test"),
 *   category = @Translation("Development")
 * )
 */
class ThemeTestBlock extends BlockBase {

  /**
   * {@inheritdoc}
   */
  public function build(): array {
    $this->messenger()->addMessage($this->t('This is an info <a href="#">message</a>.'), 'info');
    $this->messenger()->addMessage($this->t('This is a success <a href="#">message</a>.'));
    $this->messenger()->addStatus($this->t('This is another success <a href="#">message</a>.'));
    $this->messenger()->addWarning($this->t('This is a warning <a href="#">message</a>.'));
    $this->messenger()->addError($this->t('This is an error <a href="#">message</a>.'));
    return [];
  }

}

When I use this block with patching BigPipe, I can see an Info message (a new type of message supported by the Webform module), as well as other standard messages that fit my theme. When I remove the patch, I get an error

Error: The message type, info, is not present in Drupal.Message.getMessageTypeLabels().

(I tried to override getMessageTypeLabels, but couldn't. My overriding function is never called.)

And if I comment out that message line, the messages display but not with my overriden theme function Drupal.theme.message being called in my custom theme, so it doesn't fit my theme's style.

FYI, Select2 hasn't been updated for even longer than Choices. If we're going to consider ChoicesJS abandoned, the same should be said about Select2...

Is there a way to remove the unneeded settings without resaving every field manually in the UI? Or a way to automate resaving them all?

@La558 You don't quite understand. In order to use those Webform sub-modules, you need to require the libraries. For example, in your composer.json:


    "require": {
        "wikimedia/composer-merge-plugin": "^2.0"
    },
    "extra": {
        "merge-plugin": {
            "include": [
                "web/modules/contrib/webform/composer.libraries.json"
            ]
        },

You would remove this, and explicitly require the libraries you need instead.

The error is one that I fixed in Add option to disable cron activity Needs review , but if you don't want to merge that issue's fix, it can be fixed here.

The fatal error I received earlier was fixed by clearing the cache after updating. There is still an error though.

Production build 0.71.5 2024