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

Create custom sequence to access child nodes #228

Merged
merged 7 commits into from
Oct 9, 2020

Conversation

5sw
Copy link
Collaborator

@5sw 5sw commented Oct 5, 2020

Also add a method to directly return the Document from DownASTRenderable.

@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #228 into master will increase coverage by 0.04%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
+ Coverage   68.63%   68.68%   +0.04%     
==========================================
  Files          74       75       +1     
  Lines        2047     2047              
==========================================
+ Hits         1405     1406       +1     
+ Misses        642      641       -1     
Impacted Files Coverage Δ
Source/AST/Nodes/BaseNode.swift 100.00% <ø> (+9.52%) ⬆️
Tests/AST/NodeTests.swift 0.00% <0.00%> (ø)
Source/AST/Nodes/ChildSequence.swift 77.77% <77.77%> (ø)
Source/Renderers/DownASTRenderable.swift 86.66% <80.00%> (-3.34%) ⬇️
Source/AST/Nodes/Node.swift 100.00% <100.00%> (ø)
Source/AST/Visitors/Visitor.swift 92.30% <100.00%> (ø)
...rce/Renderers/DownAttributedStringRenderable.swift 50.00% <100.00%> (-4.55%) ⬇️
Tests/AST/VisitorTests.swift 98.60% <100.00%> (-0.01%) ⬇️

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 968ef0d...02f8185. Read the comment docs.

Copy link
Owner

@johnxnguyen johnxnguyen left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in the review, I started this a few days ago but forgot to submit.

For me it is good, just want to check with @iwasrobbed-ks regarding the breaking change to DownASTRenderable.

Source/Renderers/DownASTRenderable.swift Outdated Show resolved Hide resolved
Copy link
Collaborator

@iwasrobbed-ks iwasrobbed-ks left a comment

Choose a reason for hiding this comment

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

A couple of small tweaks, otherwise looks good! Thanks @5sw

Source/Renderers/DownASTRenderable.swift Outdated Show resolved Hide resolved
Source/Renderers/DownASTRenderable.swift Outdated Show resolved Hide resolved
@iwasrobbed-ks
Copy link
Collaborator

Thanks again @5sw 🎉

@iwasrobbed-ks iwasrobbed-ks merged commit 6689a8f into johnxnguyen:master Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants