KEMBAR78
Group lines by control comments by evgeniy-r · Pull Request #281 · dotenv-linter/dotenv-linter · GitHub
Skip to content

Group lines by control comments #281

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 19, 2020
Merged

Group lines by control comments #281

merged 6 commits into from
Sep 19, 2020

Conversation

@evgeniy-r
Copy link
Contributor

@evgeniy-r evgeniy-r commented Sep 11, 2020 •
edited
Loading

I changed the UnorderedKey check's logic.

Now it also splits lines into sortable groups by control comments.

This is done to make the linter logic more predictable.

Before this PR:

File:

BB=1
# dotenv-linter:off LowercaseKey
Aa=B
X=X

There is one warning (line 3).

After fixing we expect something like this:

Aa=B
BB=1
# dotenv-linter:off LowercaseKey
X=X

And we will get the incorrect file after fixing.
If we fix the Aa key somehow, it will not be the best solution either, because a user obviously doesn't want fix the lower case.

After this PR:

File:

BB=1
# dotenv-linter:off LowercaseKey
Aa=B
X=X

There is no warnings.

✔ Checklist:

  • This PR has been added to CHANGELOG.md (at the top of the list);
  • Tests for the changes have been added (for bug fixes / features);
  • Docs have been added / updated (for bug fixes / features).

@evgeniy-r evgeniy-r mentioned this pull request Sep 11, 2020
Merged
3 tasks
@mgrachev
Copy link
Member

@dotenv-linter/core What do you think about these changes in the UnorderedKey check?

@mgrachev mgrachev added the discussion Discussion of something label Sep 13, 2020
@mgrachev mgrachev requested a review from a team September 13, 2020 18:30
mgrachev
mgrachev reviewed Sep 18, 2020
View reviewed changes
Copy link
Member

@mgrachev mgrachev left a comment

Choose a reason for hiding this comment

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

@evgeniy-r Thank you for your contribution! 👍

I think we should also support the following case and don't show any warnings:

BB=1
# Simple comment
AA=B
X=X

@evgeniy-r evgeniy-r force-pushed the group_by_control_comments branch from 3f17908 to 4b0957f Compare September 19, 2020 06:24
mgrachev
mgrachev reviewed Sep 19, 2020
View reviewed changes
evgeniy-r and others added 2 commits September 19, 2020 14:52
@evgeniy-r @mgrachev
Update docs/checks/unordered_key.md
a955e3d 
Co-authored-by: Grachev Mikhail <work@mgrachev.com>
@evgeniy-r @mgrachev
Update README.md
dd5d9d6 
Co-authored-by: Grachev Mikhail <work@mgrachev.com>
@mgrachev mgrachev removed the discussion Discussion of something label Sep 19, 2020
@mgrachev mgrachev added this to the v2.2.0 milestone Sep 19, 2020
@mgrachev mgrachev mentioned this pull request Sep 19, 2020
Closed
21 tasks
@mgrachev mgrachev requested review from DDtKey and mgrachev September 19, 2020 10:03
DDtKey
DDtKey approved these changes Sep 19, 2020
View reviewed changes
Copy link
Member

@DDtKey DDtKey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for your contribution! 🚀

@DDtKey DDtKey merged commit df34f9d into dotenv-linter:master Sep 19, 2020
@mgrachev mgrachev mentioned this pull request Sep 19, 2020
Merged
3 tasks
@evgeniy-r evgeniy-r deleted the group_by_control_comments branch September 26, 2020 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgrachev mgrachev mgrachev approved these changes

@DDtKey DDtKey DDtKey approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

v2.2.0

Development

Successfully merging this pull request may close these issues.

3 participants

@evgeniy-r @mgrachev @DDtKey