Once /includes is fully converted to PSR-0, there'll be no need to scan it with the registry, since all classes will be loaded by the Symfony autoloader.

So... we should make sure we actually stop doing that before release. Critical task seems about right.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Priority: Normal » Critical
Status: Active » Needs review
FileSize
412 bytes

This patch might be enough.

Status: Needs review » Needs work

The last submitted patch, registry_no_includes.patch, failed testing.

Crell’s picture

Status: Needs work » Postponed

Agreed, although obviously we cannot do that until the current classes in /includes that rely on the registry have been migrated. Therefore marking as postponed. (Can something be critical and postponed? :-) )

pounard’s picture

Yes if it depends on another critical issues :) Fix the core so that it won't embed any scanned classes first.
Are you planning on porting DBTng to 5.3, this will the hardest task IMHO (call this madness, but I already tried @home, it's easy but very long). Saw that you actually opened a bug for this, nice.

EDIT: IMHO clean separation between procedural code and new OO code, physically I mean, seems more natural, sounds odd that all lives in includes/. I like the lib or library name for the folder containing the OO code, pretty much as a lot of other framework do.
Is there an issue for that?

catch’s picture

There's no issue for /library but if we wanted to do something like this, we should open that issue soon (i.e. before we create includes/Drupal).

pounard’s picture

I'm actually asking if you want to do something like this. It definitely feels more natural to me, but it might not to other people.

catch’s picture

I'm not sure if we should or not, it's the sort of thing that could be discussed in the issue ;)

catch’s picture

Priority: Critical » Normal
Status: Postponed » Needs review
FileSize
422 bytes

Two classes left at last count, re-rolled and running this past the testbot again.

Status: Needs review » Needs work

The last submitted patch, registry_no_includes.patch, failed testing.

catch’s picture

Status: Needs work » Needs review
FileSize
437 bytes

Oh dear.

Status: Needs review » Needs work

The last submitted patch, registry_no_includes.patch, failed testing.

franz’s picture

Status: Needs work » Needs review

#10: registry_no_includes.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, registry_no_includes.patch, failed testing.

catch’s picture

Status: Needs work » Needs review

#10: registry_no_includes.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, registry_no_includes.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review

#10: registry_no_includes.patch queued for re-testing.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Yay, passed!

I guess there isn't much to discuss anymore then. Should be RTBC.

chx’s picture

/me weeps quietly. Let's do this, yes. Let's rewrite Drupal completely.

catch’s picture

I won't weep when me move #534594: [meta] system info cache writing and registry rebuilding is broken without a full bootstrap back to 7.x because it's no longer relevant in Drupal 8. It's much easier to convert every core class to PSR-0 than it is to fix that bug properly.

chx’s picture

Did I say otherwise? Surely it's easier to rewrite everything from ground up than fixing bugs. Just why will we call this Drupal?

catch’s picture

I don't think the choice of class autoloader is what makes Drupal Drupal or not, especially since we didn't have a class autoloader until Drupal 7.

Crell’s picture

The comment of mine that chx links to is not definitive. Fortunately my fear was not entirely founded, and was based on the grotesquely poor documentation for PSR-0 that was only understandable by someone who was already used to using similar class loaders. (It's improved a bit since then.) My worry was that you couldn't have multiple roots within a single project, which would have broken modules rather catastrophically. Fortunately, I was wrong about that.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x.

Status: Fixed » Closed (fixed)

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