KEMBAR78
Update logic for IncorrectDelimiterChecker by baile320 · Pull Request #267 · dotenv-linter/dotenv-linter · GitHub
Skip to content

Conversation

@baile320
Copy link
Contributor

@baile320 baile320 commented Aug 19, 2020

I ran into issues with IncorrectDelimiterChecker: it thought, for example, "*FOO=BAR" had an incorrect delimiter (this type of error will ultimately fixed by LeadingCharacterFixer, see: #259). IMO, that is not a delimiter at all, since delimiters should occur /between/ other characters, so I updated the logic of this checker and updated some of the tests & added new ones.

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

@baile320 baile320 marked this pull request as ready for review August 19, 2020 14:06
@mgrachev mgrachev requested a review from a team August 20, 2020 09:45
@mgrachev mgrachev mentioned this pull request Aug 20, 2020
21 tasks
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.

@baile320 Thank you for your help! 👍

I have left some comments. Please look at them 👀

@mgrachev mgrachev requested a review from a team August 20, 2020 10:17
@baile320 baile320 requested review from DDtKey and mgrachev August 20, 2020 12:23
@baile320
Copy link
Contributor Author

Thanks everyone for your patience! I have made the requested 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.

LGTM 👍

@mgrachev mgrachev merged commit 54fb67e into dotenv-linter:master Aug 20, 2020
@mgrachev
Copy link
Member

@baile320 Thanks a lot! You're awesome! ❤️

🙏 If it’s not difficult for you, please support the project - click on the star on GitHub ⭐️

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants