catalyst / catalyst-moodle-workflows

4 stars 9 forks source link

Fix phpdoc check #64

Closed keevan closed 2 years ago

keevan commented 2 years ago

Currently it's not failing on errors, and also it is - in my humble opinion - painful to read. The suggested change here is to:

I have a modified version of this locally, which outputs only the things that need actioning, and removes certain things I do not deem to be unrecoverable errors. (e.g. using anonymous classes with a customised __construct.

It would be good to include those checks also into this CI.

keevan commented 2 years ago

Upstream logged issue: https://github.com/moodlehq/moodle-local_moodlecheck/issues/91

brendanheywood commented 2 years ago

Rather than just trimming the output how about completely parsing it and turning into GitHub ci error syntax

::error file={name},line={line},endLine={endLine},title={title}::{message}

https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message

the same can be said about all of the checks

keevan commented 2 years ago

Thanks for the link, that option does sounds interesting. The docs don't really provide a nice example showing what it would look like, but it would be great if the annotations appeared inline similar to performing a code review. Except it's not permanent in that if it's fixed, it simple ceases to exist.

I would probably split that into it's own issue though, and probably list it as a parent issue for all the different checks we want to convert it for.

The trimming is probably going to happen either way, and though it feels like it should be fixed upstream. The changes I had in mind are trivial and would continue to work even if upstream did fix it.