search

a13xh7 / QaraTMS

QaraTMS is open source test case, test suites, test plans and test runs management tool.
MIT License
73 stars 20 forks source link

Add laravel ide helper #34

Closed ENG3PLabs closed 3 months ago

ENG3PLabs commented 3 months ago

Add one dependency and published its config file. The dependency provides commands to generate PHP Docs for Laravel Facades and Models.

I ran an initial round of the commands to have the docs created inside a dedicated directory which is scanned by PhpStorm.

It is possible that there are methods regarding relations that are not correct if the project would be usingcustom relationship types (see https://github.com/barryvdh/laravel-ide-helper?tab=readme-ov-file#custom-relationship-types). If that's the case, please let me know. I'm not sure if I can sufficiently check on my own, since I'm still new to Laravel.

The additions were made using the -M(ixin) flag, to be able to prevent annoying IDE warnings for inconclusive type references but still keep changes to the original model files to a minimum.

Alternatively to this whole changeset, we can also force other maintainers to generate these files by themselves. However that would mean that we need to be very cauteous that the PHP docs of the models are not being altered by the script on any commit/pull request.

ENG3PLabs commented 3 months ago

I added the types of the variables to the blade files for the repository view to demonstrate the behavior of the IDE. Feel free to check it out. The benefit comes in play when trying to use data from a model, since PhpStorm can now autocomplete the fields.

It is not perfect though as for example inside the test_cases_list_with_child_suites view, PhpStorm does not realize that $childSuite is a $suite just by adding the type to $suite. This however is due to the lack of proper return types from the HasAdjajencyList class. I opted to only declare the variables injected through the controller and not local ones. The local ones I'd do inline.

Edit: I also just did that for demonstration purposes.

ENG3PLabs commented 3 months ago

@a13xh7 what do you think, is this worth it? I'd also try to document the handling of the commands somewhere to know what to do when a model got updated, since the helper files need to be regenerated in that case.

ENG3PLabs commented 3 months ago

I tested the workflow for adding a new field to a model and regenerating the PHP-docs. Unfortunately this marks the models inside ./app/ as changed although the content of the files is not altered. I checked the source of the laravel-ide-helper project and indeed, it writes to the models even if the "nowrite" option is set.

Q1: Is it acceptable to have files marked as changed on a commit even though there is no real change of the content inside the files?

Q2: Where should I document this? I don't think it belongs into the README.md.

a13xh7 commented 3 months ago

@ENG3PLabs I don't have any experience with laravel ide helper, so do as you think is right.

  1. yes
  2. README.md is ok, at the end of the file.
ENG3PLabs commented 3 months ago

Updated the readme. Regarding the changes, it was a "glitch" since I work on Windows and the tool generates unix line endings. This lead to all the files that it touched having unix line endings instead of Windows which got picked up by git but was not shown as a change inside the diff. Commiting the files however fixes the line endings since I have git configured to do autocrlf.