Skip to content

Support globs in c-sources#123

Merged
sol merged 1 commit intosol:masterfrom
mitchellwrosen:master
Sep 11, 2016
Merged

Support globs in c-sources#123
sol merged 1 commit intosol:masterfrom
mitchellwrosen:master

Conversation

@mitchellwrosen
Copy link
Copy Markdown
Collaborator

@mitchellwrosen mitchellwrosen commented Aug 14, 2016 •
edited
Loading

@mitchellwrosen
Copy link
Copy Markdown
Collaborator Author

mitchellwrosen commented Aug 14, 2016

This is ready for review, the CI errors are all aeson-1.0.0.0 related.

@mitchellwrosen
Copy link
Copy Markdown
Collaborator Author

mitchellwrosen commented Aug 26, 2016

Anything else I should do for this @sol?

@sol
Copy link
Copy Markdown
Owner

sol commented Aug 29, 2016

Sorry for being lazy on this. Can you rebase this on current master, and I will try to take care of this soon.

@mitchellwrosen
Copy link
Copy Markdown
Collaborator Author

mitchellwrosen commented Aug 29, 2016

No worries! Rebased.

@sol
Copy link
Copy Markdown
Owner

sol commented Sep 10, 2016

Hey Mitchell, sorry again for the delay. I am currently working on a voice programming set up, and I hope this will allow me to avoid situations like that in the future.

Regarding your code, I would try to make the code for the main feature more concise. I may try some things here myself.

But some things you did are useful on their own. Let's split them out into a separate PR.

sol
sol reviewed Sep 10, 2016
View reviewed changes
src/Hpack/Config.hs

(dataFilesWarnings, dataFiles) <-
expandGlobs dir (fromMaybeList packageConfigDataFiles)
expandGlobs "data-files" dir (fromMaybeList packageConfigDataFiles)
Copy link
Copy Markdown
Owner

@sol sol Sep 10, 2016

Choose a reason for hiding this comment

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

Can you move this into a separate commit?

@sol sol merged commit 48abb4a into sol:master Sep 11, 2016
@sol
Copy link
Copy Markdown
Owner

sol commented Sep 11, 2016

I applied some refactoring on top of your changes. To see the total diff you may want to look at #131.

@yamadapc yamadapc mentioned this pull request Jan 11, 2017
Closed
liskin added a commit to liskin/hpack that referenced this pull request Jan 28, 2017
@liskin
Mention c-sources globbing (sol#123) in README.md
13a6d17
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.

2 participants

@mitchellwrosen @sol