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.

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

Use literal block style for multi-line YAML #131

Merged
merged 1 commit into from
Feb 15, 2020

Conversation

omus
Copy link
Contributor

@omus omus commented Jan 23, 2020

Allows for Julia code to not require the use of semi-colon which is matches how most users will write Julia code. An additional benefit is that it allows for use of single line comments.

@omus omus requested a review from christopher-dG February 11, 2020 15:55
@christopher-dG
Copy link
Member

Thanks for the bump, I'll get to this on the weekend.

Copy link
Member

@christopher-dG christopher-dG left a comment

Choose a reason for hiding this comment

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

Looks like a good idea. I think the GitHubActions plugin could benefit from this as well.
Also, the test files will need to be updated (you can include("test/runtests.jl") and accept the prompts to update the files).

Allows for Julia code to not require the use of semi-colon which is
matches how most users will write Julia code. An additional benefit
is that it allows for use of single line comments.
@omus omus force-pushed the cv/yaml-literal-block branch from 383b595 to e258aac Compare February 14, 2020 18:09
@codecov
Copy link

codecov bot commented Feb 14, 2020

Codecov Report

Merging #131 into master will increase coverage by 1.26%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage   92.04%   93.31%   +1.26%     
==========================================
  Files          17       17              
  Lines         327      329       +2     
==========================================
+ Hits          301      307       +6     
+ Misses         26       22       -4     
Impacted Files Coverage Δ
src/plugins/src_dir.jl 87.50% <0.00%> (-12.50%) ⬇️
src/template.jl 100.00% <0.00%> (ø) ⬆️
src/plugins/git.jl 91.11% <0.00%> (+4.74%) ⬆️
src/plugin.jl 93.93% <0.00%> (+6.06%) ⬆️
src/plugins/project_file.jl 87.50% <0.00%> (+12.50%) ⬆️

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 ab9949a...e258aac. Read the comment docs.

@omus
Copy link
Contributor Author

omus commented Feb 14, 2020

Also, the test files will need to be updated (you can include("test/runtests.jl") and accept the prompts to update the files).

It would be good to document this in a CONTRIBUTING.md file

@@ -12,7 +12,7 @@ julia:
before_script:
- git config --global user.name Tester
- git config --global user.email [email protected]
script: travis_wait julia --project -e 'using Pkg; Pkg.test(; coverage=true)'
script: travis_wait julia --project -e 'using Pkg; Pkg.test(coverage=true)'
Copy link
Member

Choose a reason for hiding this comment

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

I generally like using semicolons for all keyword arguments but these changes are fine with me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also tend to use semicolons. In this particular case it seems worthwhile to be a bit more terse

@christopher-dG christopher-dG merged commit ba14f89 into master Feb 15, 2020
@christopher-dG christopher-dG deleted the cv/yaml-literal-block branch February 15, 2020 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants