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.

Sign up for GitHub

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

Jump to bottom

fix for issue #385 - support foo.md inside _annotations #387

Merged
merged 3 commits into from
Jul 14, 2016
Merged

fix for issue #385 - support foo.md inside _annotations #387

merged 3 commits into from
Jul 14, 2016

Conversation

stevemartin
Copy link
Contributor

@stevemartin stevemartin commented Jul 11, 2016 •
edited
Loading

Addresses #385

Summary of changes:

  • read in *.md files ( not recursive )
  • clobber annotations and write
  • satisfies existing tests

@tevio
fix for issue pattern-lab#385 - support foo.md inside _annotations
b448484 
* read in *.md files ( not recursive )
* clobber annotations and write
* satisfies existing tests
@tevio
refactor markdown parser code
d2c4311 
* extract functions
* add test case for zero files
@bmuenzenmeyer
Copy link
Member

@stevemartin thank you SO much for taking the time to bake this in. It's contributions and interest like yours that keep the project going.

I noticed you added a dependency, readdir, which seems at first glance to offer the same functionality as an existing dependency glob. (This would also need be added as a dependency, not a devDependency, FWIW).

I want to accept this PR as it adds measurable improvement to the library, but I'd like to see glob used or readdir used exclusively for performance reasons. I lean toward glob just from our historic usage of it, but I'll leave it up to you if willing.

Thanks again!

@stevemartin
Copy link
Contributor Author

stevemartin commented Jul 12, 2016 •
edited
Loading

@bmuenzenmeyer thanks for pointing out glob, makes total sense. I've removed the readdir dependency.

Thanks for patternlab-node!

@bmuenzenmeyer bmuenzenmeyer merged commit 6e47793 into pattern-lab:dev Jul 14, 2016
@bmuenzenmeyer
Copy link
Member

Thanks again for this @stevemartin! It will make it into next release - probably some time next week.

@bmuenzenmeyer bmuenzenmeyer mentioned this pull request Jul 18, 2016
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stevemartin @bmuenzenmeyer @tevio