njaaazi/crafty-cli

Hi there 👋

crafty-cli's Issues

Feedback

Key features / expectations

• Setup guide ✅
• Table output for humans / json output as file ✅
• Table output is readable ❌
• --output option excepts a path to a file ❌ (you did prompt instead)
• --order option ASC|DESC ❌ (you did a --ASC option instead)
• Sorting works on the full dataset (~1.600 records) ❌ (yours breaks at --limit=101)
• Packagist.org API best practices ❌ (User-Agent header missing)
• Value Object CraftPluginPackage is actually used ❌
• testLibrary property contains test lib dependency name ❌
• Some Unit test ✅ , but
  • ⚡️ tests don't work out of the box (vendor/bin/pest)
  • ⚡️ only one test works
  • ⚡️ another tests hangs

Code architecture

There is only one single class for data access, business logic, and output formatting - the PackagesCommand.

The responsibility of a command (or controller) is delegation. But here it contains almost everything!

For future applications, even if it is a tiny one like this, consider writing small classes and with limited responsibilities. Separation of concerns is the high-level term in computer science for this. If you need concrete examples in PHP, have a look at this repo:

https://github.com/jupeter/clean-code-php

Following SOLID principles and Clean Code best practices (there is much overlap), helps to maintain the code in the long run and to write Unit tests that cover small bits of the functionality. It also helps other developers to understand your code better.

Quality

• A lot of features/requirements are not implemented according to the spec
• There are many leftovers from the laravel-zero and pestphp boilerplate
• According to your README.md and composer.json the package works with PHP ^7.3 | ^8.0, but you use PHP 8.0 syntax
• Test coverage is not sufficient