Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drupal 9 #58

Merged
merged 5 commits into from
Nov 16, 2020
Merged

Drupal 9 #58

merged 5 commits into from
Nov 16, 2020

Conversation

seth-shaw-unlv
Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv commented Nov 10, 2020

GitHub Issue: Resolves Islandora/documentation#1679

What does this Pull Request do?

Drupal 9 readiness.

What's new?

  • Changes deprecated methods for the new ones (courtesy of rector).
  • Updates minor versions of required modules in composer to the Drupal-9 versions.
  • Added core_version_requirement: ^8 || ^9.

How should this be tested?

  • Apply the PR
  • composer require drupal/geolocation:^3.2 drupal/token:^1.7
  • Run upgrade_status to ensure no known issues found: composer require 'drupal/upgrade_status:^3.0' && drush en -y devel upgrade_status && drush us-a controlled_access_terms

Interested parties

@Islandora/8-x-committers, esp @kayakr

@seth-shaw-unlv
Copy link
Contributor Author

Any takers, @Islandora/8-x-committers?

@elizoller
Copy link
Member

i can give it a whirl if you'd like - anything in particular i should look for?

@seth-shaw-unlv
Copy link
Contributor Author

Nope. It should be ready to go. I just need a Drupal 9 compatible release so I can get my ArchivesSpace module updated too.

@seth-shaw-unlv
Copy link
Contributor Author

Although a smoke test never hurts!

@elizoller
Copy link
Member

Upgrade status is returning 2 warnings:

Controlled Access Terms, --
Scanned on Mon, 11/16/2020 - 15:42

FILE:
web/modules/contrib/controlled_access_terms/tests/src/Kernel/EdtfUtilsTest.php

STATUS         LINE                           MESSAGE
--------------------------------------------------------------------------------
Check manually      Class PHPUnit\Framework\TestCase not found.
--------------------------------------------------------------------------------
Check manually 15   Class PHPUnit\Framework\TestCase not found.
--------------------------------------------------------------------------------

@seth-shaw-unlv
Copy link
Contributor Author

Huh... I wonder how it missed that before. 😕

Thanks.... I'll see what I can do.

@seth-shaw-unlv
Copy link
Contributor Author

@elizoller, do you have devel installed on the test box? See https://www.drupal.org/project/upgrade_status/issues/3137754.

@kayakr
Copy link
Contributor

kayakr commented Nov 16, 2020

@seth-shaw-unlv The changes visible at Files changed make sense with respect to compatibility to Drupal 9 but I look at the composer patch I'd need to apply https://patch-diff.githubusercontent.com/raw/Islandora/controlled_access_terms/pull/58.patch it includes a bunch of other changes around controlled_access_terms_defaults config yml's and field changes. Am I driving composer wrong?

@elizoller
Copy link
Member

i did drush pm-uninstall devel and re-ran drush us-a controlled_access_terms and got the same output - is that what you meant by having 'devel' installed?

@seth-shaw-unlv
Copy link
Contributor Author

@kayakr, I believe your patch was based on an older commit. The master→main commit that contains a bunch of these changes was back in September.

@seth-shaw-unlv
Copy link
Contributor Author

@elizoller, huh... no devel should be installed: drush en -y devel.

@elizoller
Copy link
Member

devel was enabled before. did drush pm-enable devel and drush us-a controlled_access_terms and got the same error. i'll try another build.

@kayakr
Copy link
Contributor

kayakr commented Nov 16, 2020

@kayakr, I believe your patch was based on an older commit. The master→main commit that contains a bunch of these changes was back in September.

https://patch-diff.githubusercontent.com/raw/Islandora/controlled_access_terms/pull/58.patch is a patch based on your current PR. What I'm seeing is the changes included in 8.x-1.x...seth-shaw-unlv:8.x-1.x - did you rebase your fork before making the Drupal 9 changes?

@seth-shaw-unlv
Copy link
Contributor Author

@kayakr, oh! I see that now. I thought I had rebased my branch before I made the changes... and the github interface is only showing what I changed... but maybe I didn't rebase and Github is glossing over that.

@dflitner
Copy link

dflitner commented Nov 16, 2020

Controlled Access Terms, --
Scanned on Mon, 11/16/2020 - 16:03

No known issues found.

This is with the Islandora box from dev.

@seth-shaw-unlv
Copy link
Contributor Author

Ah, I simply did a merge upstream (see 720e64e), instead of a rebase.

@seth-shaw-unlv
Copy link
Contributor Author

Controlled Access Terms, --
Scanned on Mon, 11/16/2020 - 16:03

No known issues found.

This is with the Islandora box from dev.

🎉 At least it isn't just me. 😅

@elizoller
Copy link
Member

then ignore me - debbie, review and merge?

@dflitner dflitner merged commit ebb2726 into Islandora:8.x-1.x Nov 16, 2020
@seth-shaw-unlv
Copy link
Contributor Author

Ah, for future reference, @elizoller,

Adding drupal/core-dev

In order to be able to run Drupal core's PHPUnit test suite, you will have to install an additional metapackage, drupal/core-dev.

(See https://www.drupal.org/docs/develop/using-composer/starting-a-site-using-drupal-composer-project-templates#s-adding-drupalcore-dev.)

@seth-shaw-unlv
Copy link
Contributor Author

Thanks, @dflitner!

@elizoller
Copy link
Member

thank you @seth-shaw-unlv that did the trick
and thanks @dflitner for swooping in with the merge

@seth-shaw-unlv seth-shaw-unlv deleted the drupal-9 branch November 16, 2020 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Controlled Access Terms for Drupal 9.x
4 participants