Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix phpdoc check #64

Closed
5 tasks done
keevan opened this issue Jul 26, 2022 · 3 comments · Fixed by #69
Closed
5 tasks done

Fix phpdoc check #64

keevan opened this issue Jul 26, 2022 · 3 comments · Fixed by #69
Assignees

Comments

@keevan
Copy link
Contributor

keevan commented Jul 26, 2022

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

  • ensure the phpdoc actually returns an error when things aren't working.
  • have an option to skip the phpdoc check if one doesn't already exist
  • preserve any expected functionality
  • trim the output to only show relevant files and errors
  • remove errors relating specifically to certain anonymous class usage - which I believe is actually a bug.

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
Copy link
Contributor Author

keevan commented Aug 1, 2022

Upstream logged issue:
moodlehq/moodle-local_moodlecheck#91

@brendanheywood
Copy link
Contributor

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
Copy link
Contributor Author

keevan commented Aug 1, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants