Account created on 17 March 2011, almost 14 years ago
  • Technical Team Lead, Customer Support at Vardot 
#

Merge Requests

More

Recent comments

🇺🇸United States lhridley

@steven-snedker The project page has been updated to link to the current version's README file, and documentation has been cleaned up a bit.

I'm happy to take any constructive criticism on the documentation if you'd like to provide replacement text for the current README, I can generate a patch and update it, providing credit for the updates to you.

🇺🇸United States lhridley

Fixed in Release 2.2

🇺🇸United States lhridley

2.2.x will not be updated for D11 compatibility. It has been updated to get all unit tests running and will sunset when Drupal 10 reaches EOL. the 2.3.x branch will be the first branch compatible with D11. The work on 2.3.x-dev is currently testing as compatible, some functional testing remains before an alpha release will be cut.

Closing this issue as wont'fix.

🇺🇸United States lhridley

Fix in version 2.2.x, will need to be ported to 2.3.x+.

Also test coverage will need to be expanded to cover the new classes that have been added.

🇺🇸United States lhridley

@xurizaemon I've also got tests working on the 2.2.x branch, those were pushed up earlier today. I'm using this as a baseline for the 2.3.x refactor that is currently not working.

The following test coverage does not exist for the 2.2.x or 2.3.x branches, listed in priority order:

* https://www.drupal.org/project/flysystem/issues/3499242 📌 Create a Functional test for replacing Drupal Core's "assets://" stream wrapper with a Flysystem implementation. Active (highest priority, can be implemented for 2.2.x or 2.3.x branch)
* https://www.drupal.org/project/flysystem/issues/3497811 📌 Add test coverage for Drupal\flysystem\Form\FieldMigration Active (exists on the 2.3.x branch only)

I really want to have the first set of tests covering using a Flysystem adapter to replace Drupal Core's assets stream wrapper written and working before embarking on the D11 compatibility, as well as picking back up the 3.0.x refactor to implement League/Flysystem v3 Library.

But, that is a significant body of work, so for expediency I was going to plan on implementing D11 compatibility with League/Flysystem v1 in 2.3.x, and save upgrading to League/Flysystem v3 for a 3.0.x release.

If you'd like to jump in on the functional tests for replacing the Core "assets" stream wrapper with a Flysystem Stream Wrapper (the Local adapter is included in the Drupal Flysystem main module) that would be super awesome.

🇺🇸United States lhridley

@xurizaemon -- if you can fix the linting and cspell checks please do so; however the phpstan issues are related to some functional bugs that I introduced when in pulled in the Twistor/FlysystemStreamWrapper code into the project and refactored it to remove all of the unnecessary static calls. I am tracking those issues down now, no need for both of us to work on the same thing.

🇺🇸United States lhridley

@xurizaemon Pursuant to your original posts about removing the CollectionOptimizer tests, I have spent a few hours reviewing the implementaiton of the Lazy asset optimizers in Drupal Core, and their intended use vs the Flysystem asset optimizers, and have reached the same conclusion, that the Lazy optimizers are not needed once the core Lazy optimizers are implemented, as their sole intention is to consolidate and write optimized css and js files (with "optimized" meaning minified and aggregated css and js files) to the "assets://" stream wrapper. Defining a replacement stream wrapper for "assets://" is within the purview of Flysystem, and perhaps a better use of effort would. be to evaluate whether a Flysystem adapter can be used as a replacement for Drupal Core's Asset Stream Wrapper.

I'm still in the camp of removing the Lazy optimizers and any associated tests from Flysystem, as they don't appear to be needed. However, a functional test that replaces the core "assets://" stream wrapper with a Flysystem stream wrapper does seem to be a more relevant set of tests to me, to evaluate that in fact replacing the core stream wrapper does in fact work properly.

Therefore, I'm closing this issue again, and opening an issue to create a functional test for replacing the defined core "assets://" stream wrapper with a Flysystem implementation using a Flysystem adapter.

🇺🇸United States lhridley

Final update: didn't really receive any guidance, but a discussion in Slack was helpful. I decided to evaluate the Redirect module, which is the only module I was able to locate that actually used this key value pair in code, and given that this is a configuration setting for routes, I'm going with that as my assumption for correctness.

Redirect sets the same value in the same context as a boolean and evaluates it as such, so boolean it is.

@slasher13, I pushed a correction.

🇺🇸United States lhridley

No clarification yet, but in searching through projects that we support at work, found the following similar implementation for another contrib module:

ckeditor5_icons module: https://git.drupalcode.org/project/ckeditor5_icons/-/blob/1.x/src/Routin...

public function getRoutes() {
    return [
      'ckeditor5_icons.fontawesome6_metadata' => new Route(
        '/' . $this->service->getFontAwesomeMetadataPath('6'),
        [
          '_controller' => 'Drupal\ckeditor5_icons\Controller\MetadataController::getFontAwesome6MetadataResponse',
          '_format' => 'json',
          '_disable_route_normalizer' => 'TRUE',
        ],
        [
          '_csrf_token' => 'TRUE',
        ]
      ),
     ....
    ];

If no clarification is received, I'll revise the committed work to remove the single quotes from _maintenance_access, but will leave them in place for _disable_route_normalizer.

I hate inconsistencies ... drives my OCD insane :)

🇺🇸United States lhridley

@slasher13 -- looking into this further, I'm finding conflicting documentation on this so I've reached out for clarification.

🇺🇸United States lhridley

Expanded context of issue for all defined key / values.

🇺🇸United States lhridley

Need to address failing unit tests.

🇺🇸United States lhridley

Closing as outdated, since this is for a previous unsupported version of the module as well as a previous now unsupported version of Drupal. In addition, the dependencies for Twistor\FlysystemStreamWrapper have been factored out of the module in version 2.3.x.

🇺🇸United States lhridley

Closing as outdated. Drupal Core now has the "assets" streamwrapper to manage CSS / JS assets for a site, which theoretically can be overridden to utilize a Flysystem adapter to leverage an alternate storage mechanism for css / js assets, separately from uploaded assets like images, docs, etc.

🇺🇸United States lhridley

Checked the Flysystem V1 library documentation,, there is no mention of a "swift adapter" in the list of adapters.

https://flysystem.thephpleague.com/v1/docs/

Without more information this issue cannot be addressed.

Using Flysystem v2 with flysystem_s3, local, and sftp adapters, uploading and downloading .docx and .xls files works fine.

Marking postponed, needing more information.

🇺🇸United States lhridley

Closing as outdated. The module is still in sandbox mode.

🇺🇸United States lhridley

Closing this issue as outdated. In looking through the codebase for Simple oAuth, from 8.x-1.x through to 6.0.x, the class Drupal\simple_oauth\Service\Filesystem\Filesystem does not exist.

🇺🇸United States lhridley

Ported original patch to apply against 2.3.x.

🇺🇸United States lhridley

Postponing until module is ready for D11, work is in progress.

🇺🇸United States lhridley

@xurizaemon: Thank you for the .gitlab-ci.yml file, I had not had time to track that down in the Drupal documentation to get tests running on the Gitlab CI pipeline again.

See my comments regarding the removal of the Asset classes from Flysystem, those were implemented to allow writing CSS and JS assets to a remote stream wrapper as part of a Flysystem schema implementation. With the introduction of the "asset://" scheme in Drupal core for CS/JS asset management, the can now be overridden with a normal scheme definition for Flysystem, just as you can override the "public://" and "private://" schemes.

Thank you for jumping in on this!

Production build 0.71.5 2024