Add core_version_requirement: ^8 || ^9 to build_hooks.info.yml to designate that the module is compatible with Drupal 9. See https://drupal.org/node/3070687.

CommentFileSizeAuthor
#19 3153199-19.patch7.82 KBlarowlan
#19 3153199-interdiff-19.txt499 byteslarowlan
#17 3153199-interdiff-10.txt5.83 KBlarowlan
#17 3153199-17.patch7.84 KBlarowlan
#15 3153199.patch6.47 KBlarowlan
#15 3153199-fail.patch1.18 KBlarowlan
#5 3153199-5.patch1.43 KBsuresh prabhu parkala
#10 interdiff_5-10.txt490 bytessuresh prabhu parkala
#2 3153199-2.patch368 byteskarishmaamin
#10 3153199-10.patch2.01 KBsuresh prabhu parkala

Comments

karishmaamin created an issue. See original summary.

karishmaamin’s picture

karishmaamin commented
Status: Active » Needs review
StatusFileSize
new 3153199-2.patch368 bytes
ruchi-94’s picture

ruchi-94 commented
Assigned: Unassigned » ruchi-94
xaviemirmon’s picture

xaviemirmon
Primary language English
Location Brighton
commented

Hi @karishmaamin

Thanks for your patch. I've just applied it to my site but found it doesn't cover the submodules. Any chance you could generate another one that includes build_hooks_circleci and build_hook_netlify?

suresh prabhu parkala’s picture

suresh prabhu parkala commented
StatusFileSize
new 3153199-5.patch1.43 KB

Please review the patch.

xaviemirmon’s picture

xaviemirmon
Primary language English
Location Brighton
commented
Status: Needs review » Reviewed & tested by the community

Works for me @suresh-prabhu-parkala!

ruchi-94’s picture

ruchi-94 commented
Assigned: ruchi-94 » Unassigned
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work

This isn't enough to make the module compatible, see #3139733: Automated Drupal Rector fixes

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue tags: +Novice
suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3153199-10.patch2.01 KB
new interdiff_5-10.txt490 bytes

Please review!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Needs work

FrontendEnvironmentInterface still implements ConfigurablePluginInterface which has been removed - see https://www.drupal.org/node/2946161 for how to replace it

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: Drupal 9 compatibility » [PP-1] Drupal 9 compatibility
Status: Needs work » Postponed
Related issues: +#3171708: Add test coverage

Postponing this on #3171708: Add test coverage so we can tell when it is compatible

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan
Status: Postponed » Active

#3172105: There is no schema for build_hooks.settings Adds a rough test, working on this

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: [PP-1] Drupal 9 compatibility » Drupal 9 compatibility
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review
StatusFileSize
new 3153199-fail.patch1.18 KB
new 3153199.patch6.47 KB

Test showing the issue with the interface from #10

Status: Needs review » Needs work

The last submitted patch, 15: 3153199.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review
StatusFileSize
new 3153199-17.patch7.84 KB
new 3153199-interdiff-10.txt5.83 KB

There's some issues in the schema still

Fixing those

Interdiff is against 10

acbramley’s picture

acbramley commented
Status: Needs review » Needs work 
+++ b/tests/modules/build_hooks_test/build_hooks_test.info.yml
@@ -0,0 +1,6 @@
+core_version_requirement: ^8.8 || ~9.0

This should be replaced with package: Testing

Other than that, LGTM!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review
StatusFileSize
new 3153199-interdiff-19.txt499 bytes
new 3153199-19.patch7.82 KB

Thanks! I didn't even know that was possible.

Much better this way.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Saving issue credits

acbramley’s picture

acbramley commented
Status: Needs review » Reviewed & tested by the community

💥

  • larowlan committed 1e504dd on 8.x-2.x 
    Issue #3153199 by larowlan, Suresh Prabhu Parkala, karishmaamin,...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

I'll cut a new release at this point, so people can composer install the module on D9 - hopefully that will flush out any remaining issues.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.