KEMBAR78
Add method to get substitution keys to LineEntry (#379) by zotho · Pull Request #391 · dotenv-linter/dotenv-linter · GitHub
Skip to content

Conversation

zotho
Copy link
Contributor

@zotho zotho commented Mar 17, 2021

Implements #379.

I decided to change the return type of the method to:

pub fn get_substitution_key(&self) -> Result<Vec<&str>, &'static str>

because of cases like FOO=$BAR$BAZ -> ["BAR", "BAZ"]
and because there could be an errors while parsing keys.

UPD: final function signature is:

pub fn get_substitution_key(&self) -> Vec<&str>

Function should stop parsing on error and return parsed keys: FOO=$BAR ${BAZ $BAM -> ["BAR"]

✔ Checklist:

  • Commit messages have been written in Conventional Commits format;
  • 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 on the dotenv-linter.github.io (for bug fixes / features).

@zotho zotho requested a review from DDtKey March 18, 2021 21:44
Copy link
Member

@DDtKey DDtKey left a comment

Choose a reason for hiding this comment

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

I left a few more comments, please take a look at them 🙂

@zotho zotho requested a review from DDtKey March 19, 2021 19:49
Copy link
Member

@DDtKey DDtKey left a comment

Choose a reason for hiding this comment

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

I apologize for delaying the review 😅
I left a couple more comments (I think these are the last), please take a look at them.

Thank you very much for your contribution and diligence 🚀

@codecov-io
Copy link

codecov-io commented Mar 21, 2021

Codecov Report

Merging #391 (973ae74) into master (b541bd6) will decrease coverage by 0.07%.
The diff coverage is 93.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
- Coverage   95.58%   95.51%   -0.08%     
==========================================
  Files          37       37              
  Lines        2380     2474      +94     
==========================================
+ Hits         2275     2363      +88     
- Misses        105      111       +6     
Impacted Files Coverage Δ
src/common/line_entry.rs 96.32% <93.54%> (-1.69%) ⬇️

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 b541bd6...973ae74. Read the comment docs.

@zotho zotho requested a review from DDtKey March 21, 2021 23:10
@DDtKey
Copy link
Member

DDtKey commented Mar 22, 2021

And can you merge changes from dotenv-linter master branch? 🙂

@DDtKey DDtKey linked an issue Mar 22, 2021 that may be closed by this pull request
@mgrachev mgrachev added this to the v3.1.0 milestone Mar 22, 2021
DDtKey
DDtKey previously approved these changes Mar 23, 2021
@zotho zotho requested a review from DDtKey March 23, 2021 09:16
DDtKey
DDtKey previously approved these changes Mar 23, 2021
@DDtKey DDtKey requested a review from a team March 23, 2021 09:36
mgrachev
mgrachev previously approved these changes Mar 24, 2021
@mgrachev
Copy link
Member

@zotho Please resolve a conflict.

@zotho zotho dismissed stale reviews from mgrachev and DDtKey via 00ccea5 March 24, 2021 11:33
@zotho zotho requested review from DDtKey and mgrachev March 24, 2021 11:39
@mgrachev mgrachev merged commit df11d16 into dotenv-linter:master Mar 24, 2021
@mgrachev
Copy link
Member

@zotho Thank you for your help ❤️

🙏 If it’s not difficult for you, please support the project - 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.

Add method to get substitution key to LineEntry

4 participants