ESLint is linting services.yml files

That is very weird, we do even have this test -e .prettierignore || echo '*.yml' > .prettierignore.

The code that runs the job is here: https://git.drupalcode.org/project/gitlab_templates/-/blob/1.0.x/include...

The line that runs the prettier is this $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --format=junit --output-file=$CI_PROJECT_DIR/junit.xml .

I wonder if we just need to remove the yml part here --ext=.js,.yml. I think that might be it. Creating MR.

I made the change. Could you test?

You can do so changing the template references temporarily in you ".gitlab-ci.yml" to test against this fork.

include:
 - project: issue/gitlab_templates-3390343
    ref: 3390343-no-yml-check
    file:
      - '/includes/include.drupalci.main.yml'
      - '/includes/include.drupalci.variables.yml'
      - '/includes/include.drupalci.workflows.yml'

I think we should not simply ignore every .yml file. That is too drastic, there are lots of good checks that eslint performs on .yml files. We should work out what the problem is with the specific file(s) mentioned in this issue. If the config is not correct we can fix it. If certain exported .yml files always fail we can automatically add them to the .eslitnignore file. Shutting off .yml testing is surely not a good idea.

Fair enough. My first instinct was actually to mark it as "Work as designed" because the report on the syntax might be legit. I just checked the defaults in eslint and did the fix based on that, but I tend to agree with @jonathan1055 that we might miss on loads of good checks.

If something, this MR might show how to do an override on the ".gitlab-ci.yml" file, so unless there is a solid argument as per why this is a bug we might close it, but open to opinions.

Maybe @dpi can explain further what they meant by "ESLint is going off the reservation slightly by linting .services.yml files" and why it is wrong to do so. My contrib projects have .services.yml files which are linted and pass.

I made the change. Could you test?

I just went along and updated it anyway.

Maybe @dpi can explain further what they meant by "ESLint is going off the reservation slightly by linting .services.yml files" and why it is wrong to do so.

I think we should not simply ignore every .yml file.

I agree. Though ESLint is mostly going to be for JS, but we should be able to scope it to places (hint again, directories) where to expect front end files.

Porting to a new project today: it picked up a bunch of indentation issues with exported YAML config files... which I dont really care that ESLint disagrees with 🙃

I have the following in so that exported files are not checked at all:

# Config and Views exports from UI do not follow the prettier formatting rules,
# so we ignore these, rather than manually edit the exported files.
config/*/views.*.yml
tests/modules/*/config/install/*.yml

Given that the linting task is completely optional (via SKIP_ESLINT) and that there is an easy workaround (mentioned in #10) if you want to tweak the results, I think that the templates work as designed and we don't need to change anything here (happy to hear other thoughts and suggestions tho).

This will not prevent a pipeline from succeeding either.

This is "closed (works as designed)" so MR48 can be closed.

fjgarlin → closed merge request !48