Skip to content

Conversation

@mikalsande
Copy link
Contributor

When telling someone that there are too few spaces before a comment it can be useful to include how many were expected.

@coveralls
Copy link

coveralls commented Mar 14, 2025

Coverage Status

coverage: 99.815%. remained the same
when pulling a71633d on mikalsande:master
into 0f2fbb0 on adrienverge:master.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thanks for contributing!

It looks like the linter reported a line too long: can you check that?

When it's ready please squash everything into one commit (to be consistent and follow the guidelines it can be named comments: …).

expected):
yield LintProblem(comment.line_no, comment.column_no,
'too few spaces before comment')
f'too few spaces before comment, expected {expected}')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • No need for a new variable, this will be good:
                              'too few spaces before comment, expected '
                              f'{conf["min-spaces-from-content"]}')
  • Let's use a colon : instead of ,, to be consistent with other rules (like indentation or new-lines).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, fixed now.

@adrienverge
Copy link
Owner

Hello, please reread my last review entirely!

When telling someone that there are too few spaces before a comment it
can be useful to include how many were expected.
@adrienverge adrienverge merged commit 639a7c6 into adrienverge:master Apr 9, 2025
6 checks passed
@prestonvanloon
Copy link

Hit this issue today and came here to contribute exactly this. I hope it lands in a release soon, the current error message is terribly unhelpful. Thanks!

@adrienverge
Copy link
Owner

Hello Preston, I just released it in yamllint version 1.37.1.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants