[PP-1] Resolve all coding standard violations

Mostly automated fixes from phpcs -- I'm assuming we'll need more manual fixes as the test bot illuminates things.

Setting to NR to get the test bot to run.

Let's save the drupal association some costs and halt on fail for these tests to start.

Attempt to resolve some of the few remaining JS violations.

I think we'll need to disable eslint for one line -- nothing we can do about the juicebox api.

7th time's the charm, perhaps?

Okay, so #7 looks like it's passing all linting / coding standards checks that the test bot runs. The javascript took a bit of work to get passing, so we should probably manually test this with some level of scrutiny.

Luke.Leber → credited arti_parmar → .

Moved issue credits from the now-closed https://www.drupal.org/project/juicebox/issues/3371020#comment-15185053 📌 Unused variable $bundle_entity_type. Closed: duplicate

bearing in mind the adage: above all do no harm ...

3351683-coding-standards-7.patch passes the tests. If I understand the process correctly, the next step would be to create an issue fork. Then merge this. Not sure if the merge will automatically create a commit but a commit seems to be needed.

The revised code needs to get into branch 4.0.x right? At that point any developers testing this (including me) can download the revised code to their local repository(s) and test on a local set up with the latest 10.x Drupal version.

Hey Frank, it can be approached two ways. Patch workflow and issue fork workflow can go equally well.

Step one in patch workflow is to pull in 4.0.x-dev via composer (composer require drupal/juicebox:4.0.x-dev). Next, the patch needs to be downloaded locally and applied (or applied via composer patching).

Issue fork workflow can be composer require'd directly, but IIRC you'd have to set up the repository entry in your composer.json file first.

I tend to prefer patch workflow myself, just because it's a lot easier to work on a lot of different issues rapid-fire. I usually have a lot of parallel issues building when I have contrib time slots anymore.

D:\webpage\compg_2>composer require "drupal/juicebox:4.0.x-dev@dev"
./composer.json has been updated
Running composer update drupal/juicebox
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Upgrading drupal/juicebox (4.0.0-alpha1 => dev-4.0.x 99b564e)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Syncing drupal/juicebox (dev-4.0.x 99b564e) into cache
  - Removing drupal/juicebox (4.0.0-alpha1)
  - Installing drupal/juicebox (dev-4.0.x 99b564e): Cloning 99b564e1a3 from cache
Package container-interop/container-interop is abandoned, you should avoid using it. Use psr/container instead.
Package zendframework/zend-hydrator is abandoned, you should avoid using it. Use laminas/laminas-hydrator instead.
Package zendframework/zend-servicemanager is abandoned, you should avoid using it. Use laminas/laminas-servicemanager instead.
Package zendframework/zend-stdlib is abandoned, you should avoid using it. Use laminas/laminas-stdlib instead.
Package zendframework/zend-text is abandoned, you should avoid using it. Use laminas/laminas-text instead.
Package behat/mink-goutte-driver is abandoned, you should avoid using it. Use behat/mink-browserkit-driver instead.
Package fabpot/goutte is abandoned, you should avoid using it. Use symfony/browser-kit instead.
Generating autoload files
Hardening vendor directory with .htaccess and web.config files.
80 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Cleaning installed packages.
Info from https://repo.packagist.org: #StandWithUkraine
No security vulnerability advisories found.

The composer require listed (see above screen capture from Composer terminal session) is not getting the code needed. For instance the info.yml file that results is:

name: Juicebox
type: module
description: 'Provides an integration between the Juicebox HTML5 responsive gallery library and Drupal.'
package: Media
core_version_requirement: ^9.3 || ^10
configure: juicebox.admin_settings
dependencies:
  - drupal:file
  - drupal:image

with no section for the version. This info (the version) is not in the patch file. This info is in the tar.gz file that is associated with the composer require "drupal/juicebox:4.0.x-dev@dev" instruction on the page: https://www.drupal.org/project/juicebox/releases/4.0.x-dev →

A couple of days ago I had manually applied the patches in the coding standards - 7 patch but I wanted to use GIT to apply them "officially".

This process is not going very well. I have quite a few changes that I've experimented with on my local copy of the code in PHPstorm but coordinating these with whatever activity is taking place through these composer and patch processes is difficult. I think we need some kind of "target" branch where patches can be reviewed, rtbc'd and applied ... and then moved on from.

I have been continuing, as time permits to work on the set of programs related to JuiceBoxDisplayStyle.php. This necessitates changes to programs that have been already patched in this issue.

Two issues that go beyond my expertise level are

1. Replacing obsolete references to watchdog_exception (in JuiceboxXmlControllerBase.php, JuiceboxFieldFormatter.php and JuiceBoxDisplayStyle.php

and 2 in JuiceBoxGallery.php there is a function filterMarkup that attempt to make sure that only legitimate HTML codes are in the gallery object. That is seriously out of date but I'm not sure what the best approach to replacing it is. I'm sure we don't want to create a security issue by not filtering the codes in a Gallery, but am not sure how to proceed.

I will have dozens, if not hundreds of patches to apply when all this is working. Many are just typos that Phpstorm is nagging me about or sticking return values on functions or type values on parameters.

Stop overriding constructors (let's see how the test bot does).

Let's try to forget that #15 ever happened.

Ugh...and #16 too. :-)

Resolve violations added by last 2 commits..my bad there.

Postponed on https://www.drupal.org/project/juicebox/issues/3407770 📌 Move continuous integration testing to Gitlab Needs review

Since Gitlab CI is the new long term support CI provider, we should probably fix things against that versus Drupal CI.