Closed jdvivar closed 2 years ago
Merging #33 (52be542) into master (c4a9c54) will decrease coverage by
5.04%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #33 +/- ##
==========================================
- Coverage 92.68% 87.63% -5.05%
==========================================
Files 23 25 +2
Lines 164 186 +22
==========================================
+ Hits 152 163 +11
- Misses 12 23 +11
Impacted Files | Coverage Δ | |
---|---|---|
src/script.js | 37.50% <100.00%> (ø) |
|
src/main.js | 57.14% <0.00%> (ø) |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more
Thank you, @jdvivar , makes total sense and thank you for the PR and detailed explanation. It's merged, release is running.
:tada: This PR is included in version 1.10.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
I had a bad dev experience when using this plugin, that I'd like to solve with this PR. This would also have prevented this similar experience from another dev: #1
In my particular case: I'm adding the plugin, with no data at all, to a project, so that other people, at their pace, can add the actual meta data later on. I think this is a normal use case and it should work without issues. It also follows strictly the installation instructions.
I think the plugin should work straight away even when there's no data: No data, no JSON+LD.
What I've done
Using TDD I've first added a
test/nodataTest.js
which was failing as expected. Then I fixed it by returning an empty string when the plugin is called with nometa
onsrc/script.js
.I would also maybe warn users, and have some level of verbosity via an environment variable, or just a simple
console.warn
. But I didn't want to add too much logic to keep the PR simple.What do you think?