Skip to content

Improve dependency parsing (ignore comments, simplify function)#1

Closed
jzaefferer wants to merge 1 commit intojquery:masterfrom
jzaefferer:improve-js-dependency-parsing
Closed

Improve dependency parsing (ignore comments, simplify function)#1
jzaefferer wants to merge 1 commit intojquery:masterfrom
jzaefferer:improve-js-dependency-parsing

Conversation

@jzaefferer
Copy link
Member

Adds tests for nested modules and defines with line comments inside.

Copy link
Member Author

Choose a reason for hiding this comment

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

This passes as-is, but I don't think it should. Since input.js depends on version.js, their respective CSS should reflect that dependency, so this expect should be equal to "body {}\ninput {}\n"

@arschmitz @rxaviers thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've discussed this with @arschmitz and filed #2 to address this later. This PR should be good to land.

Copy link
Member

Choose a reason for hiding this comment

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

Ok

@rxaviers
Copy link
Member

Please, could you sign-off your commit?

index.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

spaces replace(/\/\/.+/g, "").

@jzaefferer jzaefferer force-pushed the improve-js-dependency-parsing branch from b4cfdb5 to 033d042 Compare October 23, 2015 15:43
Adds tests for nested modules and defines with line comments inside.

Signed-off-by: Jörn Zaefferer <joern.zaefferer@gmail.com>
@jzaefferer jzaefferer force-pushed the improve-js-dependency-parsing branch from 033d042 to 3a0bbca Compare October 23, 2015 15:45
@jzaefferer
Copy link
Member Author

Updated. Addressed all comments.

@rxaviers rxaviers closed this in dc6718e Oct 23, 2015
@rxaviers
Copy link
Member

Merged, released on 0.0.3

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