Remove HAL module from core and create a contrib project for it

If we are moving forward with this then we should create https://www.drupal.org/project/hal ASAP before someone else get some other ideas.

This is D10 material at this point. However, we should start by deprecating the module in a minor release, and then we can remove it cleanly in D10.

Mmm, also would need product signoff. FWIW I'm +1 (but not a product manager).

I provided product manager signoff in #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10. That still appears to need release manager signoff, so postponing on that.

Also parenting to the Drupal 10 module removal issue.

I think this can be unpostponed now that #3124762: Add 'lifecycle' key to .info.yml files has landed.

Happy to maintain it in contrib given entity pilot depends on it for the time being

@larowlan please create https://www.drupal.org/project/hal

https://www.drupal.org/project/hal done

Next step probably is to mark the module obsolete or just try to remove it?

Next step probably is to mark the module obsolete or just try to remove it?

Thus spoketh @andypost in #17.

I think even putting this module on lifecycle: obsolete might already be a stretch.

Looking at the "Proposed resolution" in the IS of #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10 and comment #16 in that issue, I think lifecycle: deprecated and lifecycle: obsolete for later/D10?

If that is indeed the case: MR!830 is your friend.
(Also tagged with Needs release note)

It should definitely only be deprecated in 9.x

For 10.x I think we probably need to go for direct removal instead of 'obsolete', because the course of action for a site updating to 10.x is to install the contrib module, or take a decision to uninstall the core module. Obsolete should IMO be reserved for 'not a module any more' (entity_reference, field_layout once it's merged somewhere), and development modules (simpletest).

Agree we can only deprecate in 9.x. For 10.0 we might have to leave a stub behind that can uninstall itself but is otherwise marked obsolete, like we did with simpletest; users can still install the contrib version if they want to keep the functionality.

- Changed MR!830 for marking Core Module lifecycle: deprecate from 9.3.x => 9.4.x
- Merged latest commits

Would this issue be helped out with a child [Investigation] issue like #3227033: Remove Quick Edit from core was created for the removal of QuickEdit, to see what actions/tasks/whatever would be needed to get HAL out of Drupal Core and into Contrib?

Marking this as RTBC as the one line change is trivial and there seem to be no objections to #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10

@Spokje I think that is worth trying - there will be some rearranging of tests to do as the HAL entity tests are coupled to the module that provides the entity (or core), and I guess they need moving into the HAL module.

Thanks for looking into this!

All of these issues are currently pratically postponed on #3215043: Indicate the non-stable statuses in admin/modules page, #3215045: [Duplicate] Highlight obsolete module's status at admin/reports/status page, providing warning and link with explanation so until we can resolve those, we are not making modules deperecated.

Thanks @Gábor Hojtsy for pointing out the issues that this one is postponed on. Added those as related issues.

Turns out the lifecycle_link is also required when marking a module as lifecycle: deprecate, so currently the tests in MR!830 are failing.

We need something to point that link to, so there's probably a CR needed for that.

Unsure what MR!1499 with the _exact_ same change as MR!830 but against 9.2.x instead of 9.4.x brings to the table TBH...

(Grmbl)

Restoring issue status.

Update related issues.

#3215043: Indicate the non-stable statuses in admin/modules page just got committed, only postponed now on #3250585: Highlight deprecated modules and themes at admin/reports/status page, providing warning and link with explanation.

This should only need a release note snippet, then i think we can RTBC.

I agree with the need for a release note, but the current MR (MR!830) would (at the very least) need a lifecycle_link entry in the .info.yml. I'm uncertain at this point if that should link to the Release Notes To Be, or if there's a new page somewhere on d.o. where deprecated Core modules should link to.

Besides that, there's #25 to move the corresponding tests into the correct namespace.

As an added bonus we need a maintainer for the Contrib version of the HAL module, the creation of it, moving current outstanding issues against HAL core moved to that Contrib module and a D10.0.x actual removal of the module from Core.

I'm a bit uncertain if all of the above need to be done in a follow-up issue, since I hate to see we're deprecating without the actual removal being handled.

In #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10 the removal was also approved and @larowlan offered to be maintainer of HAL and actually created the contrib module in #16. So that is kinda covered?

I personally separate the step to deprecate from the removal. But i understand that it might be scary to deprecate before rounding of the removal path.

In #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10 the removal was also approved and @larowlan offered to be maintainer of HAL and actually created the contrib module in #16. So that is kinda covered?

Ah, somehow managed to completely miss that, thanks for pointing that out @bbrala. :)

I've edited my previous comment to reflect that.
Will leave decisions on yes/no followup-issues for the removal to Bigger Brains.

It's fine to have a deprecation issue for 9.4.x/10.0.x, and then a follow-up for the removal. The deprecation is important for letting sites know it's on its way out, the removal is essentially internal to core and doesn't need all the CR/release note etc.

Ok that sounds good. I'll do the following:

1. This issue should be about the removal.
2. I'll make a child issue for the deprecation change and add a merge request.
3. I'll post in #contribute to get a rtbc on that (and ping spokje, he might come online soon ;))

Created #3263618: Deprecate HAL module.

We are still figuring the process out, but what should be more or less guaranteed to work in order:

1. Drupal ideas policy decision to deprecate and remove (in this case #3049856: [policy] Mark HAL module as deprecated in D9 so it can be removed in D10).
2. Core implementation issue opened (this one).
3. Contrib project created with at least one maintainer
4. Stable release of the contrib project ready to use on 9.4.x sites (https://www.drupal.org/project/hal is access denied for me, so we probably still need that)
4. Commit the issue to mark the module deprecated (to 9.4.x if 10.0.x wasn't open yet, or both branches if it is)
5. Commit the issue to remove the module to 10.0.x-only

I'll ping larowlan since he created the hal project

This issue should be about the removal.

Thus spoketh @bbrala in #42

- Closed deprecation D9.4.x MR
- Upped version to 10.0.x-dev

New MR (MR!1800) against 10.0.x-dev with the "fun part" of removal:

- Removal of core/modules/hal
- COMPOSER_ROOT_VERSION=10.0.x-dev composer update drupal/core -vvv

Lets see what breaks...

A lot will break, i've just checked references to the string 'hal'in core. 135 results.

Mostly in tests except: rest.module which appearantly suggests HAL in it's help.

@bbrala: Yep, I just love breaking stuff (and am quite good as it, I might add...)

As (at least I) found out in #3227033: Remove Quick Edit from core it's nice to have a record of what breaks.

Also done in that issue: Moving of tests that are _not_ in the modules namespace (which we'll see in the above failed tests) to the \Drupal\hal namespace as well as moving them into the modules directory.
This made it much easier to create a self contained Contrib module.

Seeing the amount of references to "hal" in your screenshot, it seems to me that it might be nice/wise to do the same here:
Moving "shit" (aka tests for the HAL module not in the hal-directory) around (aka move them into the hal-directory and namespace).

If it were up to me (which it isn't) I would do this in 9.4.x-dev, again just to make sure we have all and everything/kitchensinks covered and can basically move core/modules/hal into its own Contrib module and be sure all is on board.

There is something to say to move the tests in 9.4.x already. But personally I don't really have a preference. One reason to do that could be that there is less chance things changes need separate patches for d9 and d10 in the coming months :x

Moving the tests in 9.4.x is a good plan, since it'll make any potential security issue for the contrib module in 10.0.x easier to backport to core 9.4.x (although for hal specifically this is not a major concern, but in general).

If the hal project doesn't have the code in it yet, then it'll be a cleaner subtree split too.

Actually working on it here #3263654: Move all HAL tests to the module in preparation of removal in D10. Although im not pushing every commit right now :P

Postponing on moving tests in #3263654: Move all HAL tests to the module in preparation of removal in D10

Module and theme removals should always be tagged for the release notes. Thanks!

Thus spoketh @xjm in [#2966859-32]

Blocker got committed :)

Postponing on #3264633: Remove \Drupal\layout_builder\QuickEditIntegration and refactor it so that quickedit contrib provides the integration with layout builder.

Reasoning: If tests get committed in the Core hal module (See #3264633-18: Remove \Drupal\layout_builder\QuickEditIntegration and refactor it so that quickedit contrib provides the integration with layout builder), they have to be removed in MR/patch in this issue as well.

The failure of Drupal\Tests\system\Functional\System\ThemeTest was broken on HEAD, the offending commit has been reverted, #3215044: Promote the non-stable statuses in admin/appearance page, optionally even visually.

Thanks @quietone!

I also managed to create a failure that I previously resolved, so it's need some TLC anyway.
Doing so now.

No longer postponed

Forgot to change the title for the second time in 20 minutes.

I think we need to postpone this on #3263618: Deprecate HAL module which needs to go in first.

Also postponed on #3264633: Remove \Drupal\layout_builder\QuickEditIntegration and refactor it so that quickedit contrib provides the integration with layout builder (see #3049857-60: Remove HAL module from core and create a contrib project for it).

Added Currently postponed on to IS.

Since the deprecation issue is RTBC, this can go in alongside it.

I've applied this patch when testing 1.0.0 for contrib. This applied and I could use HAL from contrib without a problem after it.

Only issue is the composer.lock has of 10.0.x-dev because of the composer.json change. Hopefully this doesn't meana reroll. @larowlan is looking. ;)

Looks good to me. Reroll has merged in 10.x & the HAL module is still correctly removed.

RTBC on the assumption tests come back green

crediting @Gábor Hojtsy and @catch who've put a lot of time into this in slack etc

Added a draft change record

Committed 85ccd29 and pushed to HEAD. Thanks!

Thanks everyone, especially @Spokje and @bbrala who've championed this

Merged the change record

HAL is dead! Long live HAL¹

¹ in contrib

Update the CR to add link to the doc page and added composer instructions to the doc page. The CR mentions JSON as a superset, does that mean it is a replacement? That should be mentioned on the doc page.

🥳

There's more than one!