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

Merge Requests

More

Recent comments

That is exactly why I like the diataxis model.

The link appears to be a 404

Never mind. It appears there wasn't an argument for the logger before, so I must've just been passing a previously-unused argument.

https://git.drupalcode.org/project/ultimate_cron/-/commit/c003090f0c7352...

So it's probably true that the core of the API will work as it always has.

Even with the version supporting PHP 5.3, they say this:

If you are using PHP 5.3, you should upgrade your environment as this version has been past end of life since August 2014. Otherwise, you can download v5.9.2 (zip, tar.gz) from our releases page. This version will continue to work with new versions of the Stripe API for all common uses.

https://github.com/stripe/stripe-php?tab=readme-ov-file#legacy-version-s...

Ah. I think a 3.x branch makes sense.

I added a second MR where I upgrade the solution to work with the non-jquery version in 2.0.x branch for everyone using this and doing an upgrade.

This change is actually necessary for the accordion to work at all in the 2.0.x version if the behaviors are attached more than once. I think maybe the once function isn't working properly?

Notice that once is not added as a parameter where it should be:

  dependencies:
    - core/drupal
    - core/drupalSettings
    - core/once
/**
 * @file
 * CKEditor Accordion functionality.
 */

(function (Drupal, drupalSettings) {
  'use strict';
...
})(Drupal, drupalSettings);

I'm very minded therefore to actually introduce some kind of configuration option of how the link should be rendered, and for new views use a plain link with a 'button' class, but keep the 1.x version around as an option and allow the local actions stuff in this ticket as another option.

I think this would be a good solution.

It makes the buttons follow the site's theme and adjusts them to be themeable, so you can style them however you want. On my site, it makes the buttons look like literally every other button on the site.

I agree with #16. Now that Core is working to remove its dependencies on jQuery, that same sentiment should extend to contrib.

Using status messages doesn't work if the site doesn't use that block to show messages. I think that including the JS on every page is fine because the JS is small and doesn't really affect the load time that I can tell.

I think #22 (aka #24) should be used.

Agreed. Project Browser is still in alpha and not used by many sites yet. I think 5.1.0 should be released.

The truck is backing up in the image, so that conveys that part. But I agree that it's not easy to understand at a glance.

Okay. In that case, several contrib modules don't follow the standard and there's no coder rule to detect that, to my knowledge.

I think it would also be helpful to include guidelines and examples/suggestions regarding acronyms in class names. Sometimes you see stuff like SMTPManager, which is harder to read and looks bad, versus SmtpManager, which is preferable.

I don't have any D7 anymore, so I'm not going to follow this module or its issues anymore.

Please make any changes to the merge request rather than submitting new patches. Thanks.

I think that makes sense. Then the old service and library could be deprecated, right?

The pull request will have to be merged before sites can update if they require roave/security-advisories, since the latest release is still counted as vulnerable in the meantime.

It's also possible that the new version breaks some things due to backwards compatibility changes. I'm not sure.

Wouldn't it be better to update composer.json, so that we require a secure version? I think that's best.

As mentioned in 📌 Increase size of download buttons RTBC , I do not support these changes. I don't use the local actions block on my non-admin theme, and that makes it impossible to export view data in that case.

There should be a config option for if people want the existing behavior, rather than breaking the functionality for sites that already use the module and rely on the existing behavior.

The nginx.org recipe was outdated compared to Drupal's .htaccess file. It's probably a good thing that Nginx.org removed it.

This one looks better, at least when it comes to the "Protect files and directories from prying eyes" section:

https://github.com/uselagoon/lagoon-images/blob/main/images/nginx-drupal...

I agree that this would be incredibly useful. Accidently committing keys is a concern, and developers with less experience are more likely to assume that the default is secure, when it's not.

All that's left is to look at the thread:

Not sure why you'd want this on user.pass, I'd rather just disable this route.

https://git.drupalcode.org/project/openid_connect/-/merge_requests/98#no...

I agree. Sorting by machine name is superior. When you have weights, the weights always change, which changes them all and complicates the diffs.

I think that, for starters, it would be easiest to import a SQL file, create a backup, then check that the backup file is the same as the imported file? Then do the same with gzip enabled.

Is there already a test available to check the resulting backup for consistency? (Not being broken like before)?

I don't think so. It would be nice to have something check that the resulting backup actually works. I have occasionally found backup files that contain multiple INSERT statements with the same primary key, causing restore to fail. This could be due to site use during the creation of the backup. No test was added to ensure that the gzipped backup file is valid, either (that it doesn't contain trailing HTML, making it an invalid file).

I think it would be okay to disable taking the site offline for download, for sure, since you can easily download the file afterwards at /admin/config/development/backup_migrate/backups.

That's not true. When using Download as the destination, the backup doesn't get saved to /admin/config/development/backup_migrate/backups

You have to click the "Get Push Access" button at the top of this page.

@spadxiii Please make the change to the merge request, rather than submitting a patch.

I think secure should be the default. Unless you're an experienced Drupal developer, you're not likely to know about config overrides or the Key module. Using a key_select field as the default would be a good idea, IMHO.

It wasn't merged at the time I commented, but I'm glad it is now.

Yep, still having this issue. Selecting "Distinct" in query options doesn't fix it.

I'm still using this workaround 🐛 Duplicate row results for some data types Needs work .

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.

Production build 0.71.5 2024