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

Check errors form workflows #63

Open
hgeorgsch opened this issue Feb 1, 2021 · 13 comments · Fixed by #120
Open

Check errors form workflows #63

hgeorgsch opened this issue Feb 1, 2021 · 13 comments · Fixed by #120

Comments

@hgeorgsch
Copy link

No description provided.

@hgeorgsch
Copy link
Author

The Moodle Code Checker has been satisfied.
Many remaining problems seem to concern dummy callback functions, and although the warning can easily be fixed, there is an underlying problem of lacking error handling, which is not easily fixed.

@hgeorgsch
Copy link
Author

New errors are reported by the Code Checker.
The total number of errors is such that the work to fix them all would be tedious and demotivating.

@hgeorgsch
Copy link
Author

All workflows needs updating, using Moodle 4.x.

@jorgenfinsveen
Copy link

jorgenfinsveen commented Jul 31, 2023

Looking into this now. All of the tasks got dropped immediately and I got a warning about the Ubuntu version used in the workflow was deprecated and should be changed. Have done this and at least now all tasks are actually started before they all fail.

@hgeorgsch just an idea, but should we alter the prerequisites for task failure? E.g. the Moodle Code Checker task fails due to a warning-tolerance of 0, making the whole task fail due to e.g. an unecessary whitespace at the end of a line. Doesn't this seem a bit strict to fail the whole task if all it takes is a single warning?

@jorgenfinsveen
Copy link

Moodle Code Checker has now 0 errors but still a couple of warnings such as this:

 FILE: /home/runner/work/moodle-mod_jazzquiz/moodle-mod_jazzquiz/moodle/mod/jazzquiz/backup/moodle2/restore_jazzquiz_stepslib.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------------
 17 | WARNING | Unexpected MOODLE_INTERNAL check. No side effects or multiple artifacts detected.
    |         | (moodle.Files.MoodleInternal.MoodleInternalNotNeeded)

Because of this, the task still fails since the workflow-file specifies that there cannot be any warnings. Is this necessary?

@jorgenfinsveen
Copy link

Grunt task detects no longer any errors, but it still fails due to some warnings and due to the problem with the css mentioned in #67.

@jorgenfinsveen
Copy link

Mustache task fails due to warnings related to documentation. It will not go through until all .mustache files contain a example context such as this:

Skjermbilde 2023-07-31 kl  13 38 17

@jorgenfinsveen
Copy link

The Convert Coverage and the Coveralls Parallel tasks both fails due to missing src files. I assume that the src files is generated when running .travis.yml, but I am not sure how this could be fixed.

@hgeorgsch
Copy link
Author

Generally, we want to heed warnings, so it is definitely best if we can make the code fully compliant.
OTOH, at the moment we are overwhelmed, so if you can fix the errors, it may be a good idea to change the settings to allow warnings.

If you do turn of warnings, please make a new issue, tagged as can wait or even wont fix, with an explanation of how to turn the warning back on. Main reason for this is to give me a way to remember how to turn warnings back on.

@jorgenfinsveen
Copy link

jorgenfinsveen commented Aug 7, 2023

@hgeorgsch created an issue as you described here #119. Turned off the zero-warning tolerance in the workflow file. Workflow still fails on the CSS mentioned in #67, but that part should be all good when the pull request is approved and merged into main.

The Convert Coverage and Coveralls Parallel tasks are still failing, and I do not know enough about it to say how they can be fixed. Mustache Lint only fails on some of the tests due to the lack of documentation of the mustache-files.

Opens a pull request on the changes made so far. Leaves it open for others with more knowledge of the last failing tasks to come with some ideas on how to fix them.

@jorgenfinsveen
Copy link

jorgenfinsveen commented Aug 8, 2023

To finally complete this issue, the following needs to be done:

  • Convert Coverage and Coneralls Parallel needs to be fixed. They don't work due to a non-existing target file. I suspect that the missing file is something which is autogenerated when running the Convert Coverage task.
  • CSS !important keywords must be removed. This will be in order when removed outdated CSS #116 is merged.
  • Mustache files must have documentation specifying its template name as well as an example context as shown here.

@andstor
Copy link
Member

andstor commented Aug 11, 2023

Hi 👋 @jorgenfinsveen I can probably take a look at the Coverage file problem after I get back from vacation in a couple of weeks.

hgeorgsch added a commit that referenced this issue Aug 16, 2023
Yet to be solved: Make workflow file work again
@hgeorgsch hgeorgsch reopened this Aug 16, 2023
@hgeorgsch
Copy link
Author

See #122 and #121

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

Successfully merging a pull request may close this issue.

3 participants