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

adds min/max empty lines after document start/end #393

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wookietreiber
Copy link

@adrienverge What do you think?

Note: This currently only includes the changes to document start. Will do the changes for document end and documentation later, based on your feedback.


fixes #340

@wookietreiber wookietreiber force-pushed the document/empty-lines branch 3 times, most recently from 5622fb9 to 4c86d3f Compare July 5, 2021 09:46
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 97.528% when pulling 4c86d3f on idiv-biodiversity:document/empty-lines into 4374490 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 Christian,

This looks good!
I have a few remarks, please see my comments below.

Also, can you add a few more test cases:

  • For a document which just contains ---, nothing more?
  • In conjunction with empty-lines?
    For example, {empty-lines: {max: 1}, document-start: {min-empty-lines-after: 2} should report problems for , \n and \n\n.
  • Are there other corner cases that I didn't think of?

This currently only includes the changes to document start. Will do the changes for document end and documentation later, based on your feedback.

OK

yamllint/rules/document_start.py Outdated Show resolved Hide resolved
tests/rules/test_document_start.py Outdated Show resolved Hide resolved
yamllint/rules/document_start.py Outdated Show resolved Hide resolved
yamllint/rules/document_start.py Show resolved Hide resolved
yamllint/rules/document_start.py Outdated Show resolved Hide resolved
@wookietreiber
Copy link
Author

@adrienverge Thanks for the review. I'm gonna do the rest of the work, likely spread over the next couple of days. When I'm done, I'm going to remove the draft status and ping you for another review (dunno if removing the draft issues a notification, maybe re-requesting a review does).

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.

Thanks! I don't know about removing the draft status, but any message or comment will get me notified (however I'll may be in vacations so please accept some delay in my review...)

yamllint/rules/document_start.py Show resolved Hide resolved
@wookietreiber wookietreiber force-pushed the document/empty-lines branch 4 times, most recently from e7fef28 to 8d31c7d Compare July 14, 2021 06:57
@wookietreiber wookietreiber changed the title [document-start] adds min/max empty lines after document start/end adds min/max empty lines after document start/end Sep 24, 2021
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.

exact number of empty lines for document start/end
3 participants