Consider blank lines in UnorderedKey check #221

mgrachev · 2020-06-24T11:35:35Z

In some projects, I work with large .env files which have more than 200 lines.
Sorting these files is a really big problem.
The solution is to use groups of lines separated by blank lines. For example:

❌ Wrong
FOO=BAR
BAR=FOO

✅ Correct
BAR=FOO
FOO=BAR

✅ Correct
FOO=BAR

BAR=FOO

✔ 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).

DDtKey

This is really very useful change! 🔥

I think this should be the default behavior!
But what do you think about to make this a configurable property? 🤔

Grouped order (default) and full file order (optional)

README.md

src/checks/unordered_key.rs

mstruebing · 2020-06-25T06:52:33Z

Making it configurable is a good thing I think, but as you've stated I also think that this should be the default behaviour.
Configurability can also be handled in a separate PR to have small PRs which makes the life of everyone easier :)

DDtKey · 2020-06-25T07:52:50Z

Making it configurable is a good thing I think, but as you've stated I also think that this should be the default behaviour.
Configurability can also be handled in a separate PR to have small PRs which makes the life of everyone easier :)

Yes, of course, I am fully in favor of this being a separate PR.

codecov-commenter · 2020-06-25T10:16:43Z

Codecov Report

Merging #221 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   96.59%   96.66%   +0.06%     
==========================================
  Files          16       16              
  Lines        1469     1499      +30     
==========================================
+ Hits         1419     1449      +30     
  Misses         50       50

Impacted Files	Coverage Δ
src/checks/unordered_key.rs	`100.00% <100.00%> (ø)`
src/common.rs	`95.97% <100.00%> (+0.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09a10bb...6818eba. Read the comment docs.

Signed-off-by: Mikhail Grachev <work@mgrachev.com>

mgrachev · 2020-06-25T13:12:11Z

@DDtKey @mstruebing Configurability is a good idea, but I'm not sure that we need it in this check.
Right now, we have very simple checks that are not configurable and implementing this for one check will add extra complexity.

I would like to return to this discussion in the future if we need to implement configurability for several checks.
What do you think?

mgrachev force-pushed the update-unorderedkey-check branch 2 times, most recently from 45fa8f1 to 0d2073a Compare June 24, 2020 11:38

mgrachev requested a review from a team June 24, 2020 11:52

DDtKey reviewed Jun 24, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

mstruebing reviewed Jun 25, 2020

View reviewed changes