-
Notifications
You must be signed in to change notification settings - Fork 101
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
add PkgEval badge #334
add PkgEval badge #334
Conversation
src/plugins/readme.jl
Outdated
@@ -48,6 +48,7 @@ function view(p::Readme, t::Template, pkg::AbstractString) | |||
"HAS_CITATION" => hasplugin(t, Citation) && getplugin(t, Citation).readme, | |||
"HAS_INLINE_BADGES" => !isempty(strings) && p.inline_badges, | |||
"PKG" => pkg, | |||
"PKG1" => first(pkg) # for PkgEval badge, need first character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to use a more descriptive variable name if you can think of one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is fine with the comment :) i suppose "PKG_INITIAL" is an alternative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for missing this initially, @mileslucas, but we need to add a view
method for the PkgEvalBadge
rather than extending the Readme
method
I think this should be sufficient:
view(::PkgEvalBadge, t::Template, pkg::AbstractString) = Dict("PKG1" => first(pkg), "PKG" => pkg)
And in order to make sure this works correctly, we should add PkgEvalBadge()
to the "Wacky Options" reference tests in test/reference.jl
:
- you just need to add it to the list of plugins in the
test/reference.jl
code, then if you run the tests using Julia v1.5 (only this version runs the reference tests) it should give you the option to automatically update the actually reference files. - Of course, we then need to check the updated README.md file that gets generated has the expected badge with correct URLs.
(also xref #270)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorporated.
Note: I actually ran into issues running this in docker-
$ docker run --rm -ti -v $(pwd):/code julia:1.5 bash
# cd code
# julia --project=@.
julia>]
(PkgTemplates) pkg> test
ERROR: AssertionError: sourcepath !== nothing
So, I just changed the expected README file manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, i think ERROR: AssertionError: sourcepath !== nothing
is a cryptic Pkg.jl error, related to trying to use v1.5 withan env with a Manifest.toml generated in with Julia v1.6+ ...or something like that. Anyway, sorry it wasn'y a totally smooth process! But the manual change LGTM and CI is happy
Thanks again!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks great!
Just one more test to update i think, plus a small suggestion :)
src/plugins/readme.jl
Outdated
@@ -48,6 +48,7 @@ function view(p::Readme, t::Template, pkg::AbstractString) | |||
"HAS_CITATION" => hasplugin(t, Citation) && getplugin(t, Citation).readme, | |||
"HAS_INLINE_BADGES" => !isempty(strings) && p.inline_badges, | |||
"PKG" => pkg, | |||
"PKG1" => first(pkg) # for PkgEval badge, need first character |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is fine with the comment :) i suppose "PKG_INITIAL" is an alternative
Co-authored-by: Nick Robinson <npr251@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #334 +/- ##
==========================================
+ Coverage 94.68% 94.87% +0.18%
==========================================
Files 20 20
Lines 621 624 +3
==========================================
+ Hits 588 592 +4
+ Misses 33 32 -1
Continue to review full report at Codecov.
|
It should be ready to go! Thanks for the review. |
An example of this badge can be seen in this repo- https://github.com/JuliaPhysics/Mueller.jl