Skip to content

Conversation

@crazywhalecc
Copy link
Owner

@crazywhalecc crazywhalecc commented Aug 6, 2025

What does this PR do?

Closes #625

This PR is extremely huge, but actually haven't changed any function of building. To sum up, I divided changes into several commits, recommends that reviewing by commits.

Description of major commits:

  • Remove all @throws PHPDoc, it's almost useless for SPC anymore: Don't need to review, it just removed all @throws tag from code.
  • Refactor all exception classes, remove unclear RuntimeException, InvalidArgumentException: changes all exception classes in SPC\exception namespace.
  • Refactor shell utilities: reorganize namespaces and introduce Shell base class: Make WindowsCmd and UnixShell both extends Shell, which contains several shared methods.
  • Introduce AttributeMapper for managing extensions and doctor attributes: Prepare for 3.0, make attribute loading functions into dedicated class.
  • Refactor all (except command) modules using new exceptions: This may be quite complicated. Generally speaking, it replaces all Exception class names except those in the command class with the new class name.
  • Refactor all command class exception handling: Refactor all command exception handler.
  • Remove craft.log: After introducing log/, we don't need craft.log anymore.
  • Chore: Some code cleaning up.

As for why not 2.8.0, just because this is not introducing any additional feature for building.

Checklist before merging

If your PR involves the changes mentioned below and completed the action, please tick the corresponding option.
If a modification is not involved, please skip it directly.

  • If you modified *.php or *.json, run them locally to ensure your changes are valid:
    • composer cs-fix
    • composer analyse
    • composer test
    • bin/spc dev:sort-config
  • If it's an extension or dependency update, please ensure the following:
    • Add your test combination to src/globals/test-extensions.php.
    • If adding new or fixing bugs, add commit message containing extension test or test extensions to trigger full test suite.

@crazywhalecc crazywhalecc added kind/framework Issues related to CLI app framework mixed PR This PR contains multiple updates labels Aug 6, 2025
@crazywhalecc crazywhalecc requested review from Copilot and henderkes and removed request for henderkes August 6, 2025 13:13

This comment was marked as spam.

@henderkes
Copy link
Collaborator

It mostly looks good, but I don't quite understand the point of the AttributeMapper yet.

@crazywhalecc
Copy link
Owner Author

crazywhalecc commented Aug 7, 2025

It mostly looks good, but I don't quite understand the point of the AttributeMapper yet.

Because attribute parsing requires pre-loading, and Doctor and Extension use it, I use it simply to put the attribute loader into a single file for easier debugging. To reduce code size and optimize the structure, in 3.0 I plan to use attributes for library declarations as well.

Of course this has no real impact on functionality, it's just for future refactoring and vendor mode loading unification.

Although it is possible to directly use reflection each time a attr is used, it also requires a util class to implement it.

@crazywhalecc crazywhalecc merged commit afd6791 into main Aug 7, 2025
13 checks passed
@crazywhalecc crazywhalecc deleted the refactor/error-handling branch August 7, 2025 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/framework Issues related to CLI app framework mixed PR This PR contains multiple updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better error handling

2 participants