Skip to content

Rebase reference for static includes that target a div - #115

Merged
Gisonrg merged 2 commits into
MarkBind:masterfrom
acjh:94-rebase-ref-for-static-includes
Jan 19, 2018
Merged

Rebase reference for static includes that target a div#115
Gisonrg merged 2 commits into
MarkBind:masterfrom
acjh:94-rebase-ref-for-static-includes

Conversation

@acjh

@acjhacjh commented Jan 17, 2018

Copy link
Copy Markdown
Contributor

Fixes#94

@acjh
acjh requested a review from GisonrgJanuary 17, 2018 18:50
@acjh
acjhforce-pushed the 94-rebase-ref-for-static-includes branch from 834a11b to c9a18bbCompareJanuary 18, 2018 03:40
@acjhacjh changed the title Rebase reference for static includesRebase reference for static includes that target a divJan 18, 2018

@GisonrgGisonrg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work!

Comment threadlib/parser.js Outdated
};

Parser.prototype.includeFile = function (file, cb) {
Parser.prototype.includeFile = function (file, cb, config) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am thinking about deprecating the callback for the API and use Promise only, and I don't think we are still using callback anywhere in the codebase, so I think it's safe to change to put config after the file parameter here (do a check before you do so) :)
Let's create an issue and get rid of callback in a separate PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Checked and changed!

@acjh
acjhforce-pushed the 94-rebase-ref-for-static-includes branch from 3e0dcfa to 22cab7eCompareJanuary 19, 2018 02:00
@Gisonrg
Gisonrg merged commit c4ee372 into MarkBind:masterJan 19, 2018
@acjhacjh added this to the v1.1.6 milestone Feb 1, 2018
Sign up for freeto 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

@acjh@Gisonrg