manuel garcia โ created an issue.
Fixing a mistake, forgot to account for when we build the arguments for the command since I changed the form keys.
manuel garcia โ created an issue.
A bit of refactoring and renaming things:
1. Removed the strip-
prefix from the strip options as its unnecessary (already in the strip option array).
2. Added an all
option so this setting is explicit. This allowed for simplifying how we cook the argument later. inside applyToImage()
.
3. Adjusted the schema file accordingly.
Hello all, we were looking patch such functionality and found this issue, so collaborating.
Patch still applies cleanly and looks to work fine. I noticed that the schema file would need to be updated, so doing so in this patch.
Also ran into this today - in my case setting proper permissions on the files folder did the trick as well, so thank you @zigazou for sharing.
smustgrave โ credited Manuel Garcia โ .
I'm also facing the same issue, running drupal-check 1.4.0 and Upgrade Status 4.3.1.
Here are the contents of the neon file:
cat /tmp/upgrade_status/deprecation_testing.neon
# FROM mglaman/drupal-check/phpstan/deprecation_testing.neon
parameters:
tmpDir: '/tmp/upgrade_status/phpstan'
drupal:
drupal_root: '/var/www/html/web'
parallel:
maximumNumberOfProcesses: 0
customRulesetUsed: true
ignoreErrors:
- '#\Drupal calls should be avoided in classes, use dependency injection instead#'
- '#Plugin definitions cannot be altered.#'
- '#Missing cache backend declaration for performance.#'
- '#Plugin manager has cache backend specified but does not declare cache tags.#'
# FROM mglaman/drupal-check/phpstan/base_config.neon
reportUnmatchedIgnoredErrors: false
excludePaths:
- */tests/Drupal/Tests/Listeners/Legacy/*
- */tests/fixtures/*.php
- */settings*.php
- */bower_components/*
- */node_modules/*
Moving to review to trigger the testbot
Thank you for the report and patch, was about to open an issue for this :)
I went through the patch, and it is clean and on point. No more changes required as far as I can see.
I then applied it locally and the errors are gone, site looking functional as usual. Moving to RTBC.
It would be nice to get this in, Drupal 10.2.0 recommends using PHP 8.2 https://www.drupal.org/project/drupal/releases/10.2.0 โ
Drupal now supports PHP 8.3 and recommends at least PHP 8.2.
Thanks for suggestion. Views accordion doesn't currently ship any CSS, the styles are provided by the jquery.ui theme under modules/contrib/jquery_ui/assets/vendor/jquery.ui/themes/base/theme.css
For that to happen someone would have to write an integration with Olivero to add support for jQuery UI. jQuery UI has been marked โEnd of Lifeโ by the OpenJS Foundation - so the likely hood of this happening is very low.
Thank you everyone who helped with this, I've now comitted the changes and cut a new 3.0.0 release: https://www.drupal.org/project/contact_permissions/releases/3.0.0 โ
Let's bring in the phpstan.neon file from the PR we worked on for ๐ view_profiles_perms_test not compatible with Drupal 10 Fixed , it should take care of this warning: https://git.drupalcode.org/issue/view_profiles_perms-3406407/-/jobs/475158
Re @andrew.farquharson #8
Hi @Manuel Garcia, Since branch 2.1.x has core compatibility with Drupal 9 and 10 I am not sure if the .gitlab-ci.yml needs to be edited to test on Drupal 9 and 10 versions? I imagine that the scope of this ticket is to make changes only to the .gitlab-ci.yml. The functional test is now fixed so it should work on Drupal 9 and 10, now? Is there anything i am missing?
I'm not entirely sure if we need to customize our .gitlab-ci.yml file to be honest, I had a read of https://www.drupal.org/node/3356364 โ and couldnt figure it out. You are right in that we should be testing with both supported Drupal core versions though.
We now have a passing 2.1.x branch, so we can proceed with this.
OK patch came back green, committed and pushed to 2.1.x - thanks!
Ok I have compiled the changes form the PR #6 into the attached patch, except the changes related to gitlabci which we will add on โจ Configure GitLab CI Postponed .
Added PHPstan config file on https://git.drupalcode.org/project/view_profiles_perms/-/merge_requests/... to handle warnings https://git.drupalcode.org/project/view_profiles_perms/-/jobs/460383
$ php vendor/bin/phpstan analyze $_WEB_ROOT/modules/custom/$CI_PROJECT_NAME $PHPSTAN_CONFIGURATION --no-progress || EXIT_CODE=$?
------ ------------------------------------------------------------------------------
Line src/ViewProfilesPermsPermissions.php
------ ------------------------------------------------------------------------------
41 Unsafe usage of new static().
๐ก See:
https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static
------ ------------------------------------------------------------------------------
https://www.drupal.org/docs/develop/development-tools/phpstan/handling-u... โ for context.
thanks Andrew for the work on this, its nice to see the tests running on Gitlab CI!
On this ticket we will be committing only whats needed for Drupal 10 compatibility. Once we fix this ticket we will tackle getting the Gtlab CI
running on
โจ
Configure GitLab CI
Postponed
Regarding the error fixed by adding of the permission configuration (https://git.drupalcode.org/issue/view_profiles_perms-3406820/-/jobs/452805), I think that adding the config file is not the correct solution, as this a permission dynamically provided by view_profiles_perms module, which is what we want to test for. However lead me to investigate further, and I eventually landed on #2571235: [regression] Roles should depend on objects that are building the granted permissions โ - see its change records:
So this may technically be a actual current bug in Drupal 10? Not sure to be honest, but we probably have to mimic how \Drupal\taxonomy\TaxonomyPermissions
does things
Thanks, I'm learning about this as I go, but as far as I can tell the build failed with:
Unable to install modules: module 'view_profiles_perms_test' is incompatible with this version of Drupal core.
- and its using Drupal version : 10.1.8-dev
So indeed this is a bug, we (I) forgot to upgrade to Drupal 10 the module we use in the test to setup things.
I've created ๐ view_profiles_perms_test not compatible with Drupal 10 Fixed for the same, postponing this until we get that fixed, so we can get tests actually running on Gitlab.
Manuel Garcia โ created an issue. See original summary โ .
thanks @plopesc!
Apologies @andrew.farquharson - i have now pushed an empty commit to rectify the commit mentions for this ticket.
Manuel Garcia โ created an issue.
@andrew.farquharson this is already fixed in the 2.1.x
branch:
https://git.drupalcode.org/project/view_profiles_perms/-/blob/2.1.x/comp...
"require": {
"drupal/core": "^9 || ^10"
}
https://git.drupalcode.org/project/view_profiles_perms/-/blob/2.1.x/view...
core_version_requirement: ^9 || ^10
I've just switched the default branch of the project to 2.1.x
in order to avoid future confusion and tagged a new 2.1.1
release with this fix.
Sorry for messing up, doing things in a hurry is never a good idea. Thanks for the patch, committed and pushed to 2.1.x - Please verify and I will cut a new tag.
Thanks @ploplesc!
Could you please confirm whether this is working as expected for you in -dev version before tagging a new release?
I did that just now, my findings
- Removed the previous patch
- Switched to using
dev-2.0.x
- Ran composer install
- Cleared caches
- Ran
drush prie
- got me a valid link, serving a valid csv file. - Ran
drush prii
no errors but i didn't try actually importing a file.
No errors are coming up if I run drush status
or anything similar.
I've just cut a new 2.1.0 โ release. Added the composer.json file and also we are dropping Drupal 8 support.
Thank you all for the help and patience in this.
https://git.drupalcode.org/project/view_profiles_perms/-/commit/fe151a20...
Thank you very much @AstonVictor
is this project abandoned? I don't see much activity https://git.drupalcode.org/project/ratio_crop/-/commits/8.x-1.x?ref_type...
There is a 2.0.x branch with Drupal 10 compatibility. Any bugs related to the upgrade should be logged against that branch.
A 2.0 release would be in order, since Drupal 9 is no longer supported.
Forgot to update the namespace on previous patch...
Updated the patch with the change in directory.
I tested the #2 patch, verified the namespace and its the correct fix. The error is also gone once the change is applied.
@Ali_W, try verifying that the web/modules/contrib/migrate_tools/src/Drush/MigrateToolsCommands.php
file is actually there perhaps.
Updating the title as this isn't just PHP 7.4.
Taking the leap here and moving to RTBC, it is being reported as valid, patch is small and looks good to me. Thank you @godotislate.
Just bumping this to get the maintainers attention. Drupal 9 EOLed on November 1st and the patch is simple and good to go.
Drupal 9 EOL is just around the corner (November 1st).
The patch would get us Drupal 10 compatibility and is very simple. RTBCing.
Here are the results of running the upgrade status tool on the project:
$ drush us-a jquery_countdown_timer
[notice] Processing /var/www/html/web/modules/contrib/jquery_countdown_timer.
================================================================================
jQuery Countdown Timer, 8.x-1.3
Scanned on Fri, 10/20/2023 - 09:33
FILE:
web/modules/contrib/jquery_countdown_timer/jquery_countdown_timer.libraries.yml
STATUS LINE MESSAGE
--------------------------------------------------------------------------------
Check manually 0 The 'countdown.timer' library is depending on a deprecated
library. The core/jquery.once asset library is deprecated in
Drupal 9.3.0 and will be removed in Drupal 10.0.0. Use the
core/once library instead. See
https://www.drupal.org/node/3158256
--------------------------------------------------------------------------------
FILE: web/modules/contrib/jquery_countdown_timer/jquery_countdown_timer.info.yml
STATUS LINE MESSAGE
--------------------------------------------------------------------------------
Check manually 0 Value of core_version_requirement: ^8 || ^9 is not
compatible with the next major version of Drupal core. See
https://drupal.org/node/3070687.
--------------------------------------------------------------------------------
FILE: web/modules/contrib/jquery_countdown_timer/composer.json
STATUS LINE MESSAGE
--------------------------------------------------------------------------------
Check manually 0 The drupal/core requirement is not compatible with the next
major version of Drupal. Either remove it or update it to be
compatible. See
https://drupal.org/node/2514612#s-drupal-9-compatibility.
--------------------------------------------------------------------------------
All points picked up by the upgrade_status analisys tool have been addressed in the latest patch.
I have also gone over the changes on jquery_countdown_timer_init.js (which isn't picked up by the tool), they make sense and there is no other adjustments that we need to make there as far as I can see. To me the patch looks good to go so I am RTBCing.
Drupal 9 EOL is November 1st, which is just around the corner, it would be nice if we can get this in and cut a new release :)
hi @snap_x - just assessing where we are here (November 1st is coming very soon!), I had a look at your PR and it looks good, I only have one question though, the bot's instagram_lite.1.0.x-dev.rector.patch
also removes the dependency on core's block module, should we do that?
Thank you for working on this, I went through the PR and it all seemed reasonable, +1 to RTBC. Would be great to get this in and cut a new release as D9 EOL is coming very soon โ (November 1st!)
First, thanks all for the patch, I have been evaluating it and seems to work as advertised.
I had to reroll it so I am attaching the re-roll which should apply cleanly against 4.x
.
De-assigning assuming this is no longer being worked by gbyte
:)
Woot! #3359159: Add Status to OverviewTerms.php form was merged into core! Small step, but it improves the core UI when using this patch.
Hey thanks for that, a good improvement indeed!
Thanks very much for the offer @heddn, I have added you as a co-maintainer now.
Yeah, I believe #1863906: [PP-1] Replace content revision table with a view โ to be blocked, see my comment #56 โ there.
Reverting the changes related to #45.7, apparently we cant use a method from a trait?
OK some more progress, did all I could but most likely someone else will need to help out here :)
#45.5 Had a look, but unsure how to do this,
RevisionRouteProvider::getRevisionRevertRoute()
is already adding:
$route->addRequirements([
'_entity_access_revision' => "$entity_type_id.update",
]);
Is this where we should do this?
#45.6 I agree, replaced with {@inheritdoc}
#45.7 I dont see why not, done (I hope)
#45.8 I don't know, left as is.
#45.9 - Same deal as for #45.5
#45.11 No reason that I can think of, fixed.
#45.12 I agree.
#45.13 Done.
#45.14 Dont know :)
Thanks for the review @Sam152!
Had just a bit of time, so here's just a bit of progress on that:
- #45.1 - Yay, done (I hope)
- #45.2 - Done
- #45.3 - Possibly, changed to just using
'entity_revisions_table'
.
Setting to needs review for the tests to run, but obviously this is still Needs work
to address the rest of #45.
1) Drupal\FunctionalTests\Routing\RevisionRouteAccessTest::testRevisionRouteAccess
Unable to install modules entity_test, user, entity, block due to missing modules entity.
This should get the tests running.
Most conflicts are easy enough to fix, however core/misc/autocomplete.js has been changed by #2823589: Improve IME handling on Autocomplete โ so I'm stepping away from this reroll in fear of messing things up :)
Rerolled, was just a simple conflict on core/misc/autocomplete.js