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

Prevent 621 punct at end for UGNT ellipsis #128

Merged
merged 4 commits into from
Jan 15, 2021
Merged

Prevent 621 punct at end for UGNT ellipsis #128

merged 4 commits into from
Jan 15, 2021

Conversation

RobH123
Copy link
Contributor

@RobH123 RobH123 commented Jan 14, 2021

There are four ellipsis in the UGNT and Alan Bunning confirms that they are intentional (partial OT quotes).

Fixes 2nd part of unfoldingWord/tc-create-app#574


This change is Reviewable

@RobH123 RobH123 marked this pull request as draft January 14, 2021 18:15
@codecov
Copy link

codecov bot commented Jan 14, 2021

Codecov Report

Merging #128 (de13f61) into master (2eeee44) will increase coverage by 0.02%.
The diff coverage is 31.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   46.19%   46.21%   +0.02%     
==========================================
  Files          22       22              
  Lines        2784     2787       +3     
  Branches      893      894       +1     
==========================================
+ Hits         1286     1288       +2     
- Misses        942      943       +1     
  Partials      556      556              
Impacted Files Coverage Δ
src/core/tn-links-check.js 53.49% <27.77%> (+0.16%) ⬆️
src/core/orig-quote-check.js 63.13% <100.00%> (ø)

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 2eeee44...de13f61. Read the comment docs.

@RobH123 RobH123 marked this pull request as ready for review January 14, 2021 22:15
@RobH123
Copy link
Contributor Author

RobH123 commented Jan 14, 2021

Really, this was just a one-character fix (adding an ellipsis to a string in a comparison), but I struggled to try to find out why tests pass locally, but fail here. (Still don't fully understand it unfortunately, but they're both passing at present.)

Copy link
Contributor

@mandolyte mandolyte left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, 3 of 3 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ancientTexts-net)

@mandolyte mandolyte merged commit 24fcef6 into master Jan 15, 2021
@mandolyte mandolyte deleted the Fix621 branch January 15, 2021 13:06
@mandolyte
Copy link
Contributor

@RobH123 reviewed & merged. I'll let you publish to NPM when you are ready.

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