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.

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

finish doc comments #9

Merged
merged 2 commits into from
Nov 19, 2020
Merged

finish doc comments #9

merged 2 commits into from
Nov 19, 2020

Conversation

litenite42
Copy link
Contributor

Adds basic documentation comments (in English) and fixes a deprecated syntax warning (line 71)

Btw great work on this! I love the simplicity of dotenv and it's great to have it ported to V!

@zztkm zztkm added the documentation Improvements or additions to documentation label Nov 18, 2020
zztkm
zztkm approved these changes Nov 18, 2020
View reviewed changes
Copy link
Owner

@zztkm zztkm left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!!!
I'm glad you left Japanese :D

This PR is LGTM!

@litenite42
Copy link
Contributor Author

You can merge this whenever you'd like then. Let me know if I can be of any aid with that.

@litenite42
Add .env comment support and test
eb597d0 
handle most comment cases and update tests

noticed v fmt was off
@litenite42 litenite42 mentioned this pull request Nov 19, 2020
Closed
@litenite42
Copy link
Contributor Author

Should be good to go. Both PR's are merged here, so it should be a clean merge for you.

Should support:

TEST=OVERLOADENV
TEST2=LOADENV
#TEST3=NOLOADENV
TEST4=NOHASH#COMMENTSTARTSHERe
TEST5=NOHASH #TEST6=NOLOADENV
TEST7 = "HASH #ENV" # COMMENT

zztkm
zztkm approved these changes Nov 19, 2020
View reviewed changes
Copy link
Owner

@zztkm zztkm left a comment

Choose a reason for hiding this comment

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

lgtm!

zztkm
zztkm approved these changes Nov 19, 2020
View reviewed changes
Copy link
Owner

@zztkm zztkm left a comment

Choose a reason for hiding this comment

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

test approved!

@zztkm zztkm merged commit 7cb1735 into zztkm:main Nov 19, 2020
@zztkm
Copy link
Owner

zztkm commented Nov 19, 2020

@nyx-litenite
Thanks!

I have confirmed that all tests are OK.

i approved & merged!

@litenite42
Copy link
Contributor Author

Awesome! Glad to help :) I'll keep an eye on the roadmap issue for anything else i can help w/

@zztkm
Copy link
Owner

zztkm commented Nov 19, 2020 •
edited
Loading

@nyx-litenite
OK!

I would like to discuss the roadmap here(#11 ).
It will help you to implement what features in the future!

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

@zztkm zztkm zztkm approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@litenite42 @zztkm