Move exceptions to top-level namespace [BC break]

Currently, exceptions are in Grifart\Stateful\Exceptions, but are defined in files section of autoloading. This probably mismatch PSR-4 standard as reported here (relevant SA question).

I suggest to move exceptions to the top-level namespace so it corresponds to where the file is placed actually. We use this pattern in apps as well so there is no point in doing it other way here, I think.

Comments? @jkuchar1 @jpudil

changed title from Moved exceptions to top-level namespace [BC break] to Move exceptions to top-level namespace [BC break]

changed the description

What problem does it solve?

I think that there is no upside of doing this. As there si nothing to optimize in autoload, when exceptions.php is loaded everyting app starts.

See the linked CI job. There is plenty of warnings in production mode (--no-dev --optimize-autoloader) – that's the problem, it's confusing.

I suggest to move exceptions to the top-level namespace so it corresponds to where the file is placed actually.

Is that enough to get rid of the warnings?

I mean, if we truly want to adhere to PSR-4 then each exception has to go to its own separate file. That's insane to do in the top-level namespace because it would just pollute the source root, and I don't see the value in it under the Exceptions namespace either.

Is that enough to get rid of the warnings?

Why it shouldn't be? Once their namespace match the folder they sit in, it should match the main autoload rule "Grifart\\Stateful\\": "src/"

Hm, really, it didn't help. But I don't understand it. Why then, in CampApp, it doesn't complain when we use this exceptions.php pattern and here it does?

@dkurowski Please update the issue. This is not about moving expcetions, but fixing composer warnings.

@jkuchar1 Well, I'm not still pretty sure what is the issue actually so I have nothing to update right now.

Current summary: I want to get rid of warnings in production build (composer install --optimize-autoloader) and now we're investigating what is the root cause. @jpudil says the warnings are reasonable but I can't see why they appear in here but not in CampApp where we use the same pattern.

@dkurowski in CampApp, the exception files are autoloaded via a classmap, not directly as is the case here. We could do the same thing here I guess.

Aha, that's what I meant probably, just didn't realize difference between files and classmap. I'll try it.

We use files for dev and classmap for production, because:

• dev classes are found immediately without dumping composer autoload
• classmap is fast, as it does not include files it does not need to

So the combination is the best.

👉 !24 (merged)

mentioned in merge request !24 (merged)

assigned to @dkurowski

mentioned in commit 9d585b2a

closed with merge request !24 (merged)