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

Extract FilePattern to its own library #550

Open
eborden opened this issue Nov 13, 2017 · 16 comments
Open

Extract FilePattern to its own library #550

eborden opened this issue Nov 13, 2017 · 16 comments

Comments

@eborden
Copy link

eborden commented Nov 13, 2017

Shake's FilePattern seems like a useful module. It should be extract into a library so it can be shared by other tools.

This has been mentioned before:
#499
#485
ndmitchell/hlint#402
ndmitchell/hlint#334

@ndmitchell
Copy link
Owner

Are you going to give this a try?

@ndmitchell
Copy link
Owner

One thing to note is that Shake supports // patterns mostly as a legacy feature. I suggest the new filepattern package also supports that, but perhaps behind a useLegacyPatterns function, just so Shake can migrate more slowly.

@eborden
Copy link
Author

eborden commented Nov 13, 2017

I'm going to give it a go.

@eborden
Copy link
Author

eborden commented Nov 14, 2017

First pass to extract the necessary modules:
https://github.com/eborden/filepattern

@eborden
Copy link
Author

eborden commented Nov 14, 2017

I've done the work to provide FilePattern and FilePattern.Legacy. As well as opening #551 to begin relying on that library. There is a bit more work in filepattern getting CI running and re-enabling some tests, but should be enough draft work for you to review.

@eborden
Copy link
Author

eborden commented Nov 14, 2017

Oh also not sure how you want to handle the filepattern repo. Totally fine to hand it over to you if you'd rather keep maintainer-ship.

@ndmitchell
Copy link
Owner

Thanks for all the work, I’ll review later. What do you prefer/hope in terms of maintainership? You, me or both seems the possible options.

@eborden
Copy link
Author

eborden commented Nov 14, 2017

I think a dual maintainership sounds good. This lib is integral to shake so it is probably good for you to at least have one hand on the wheel.

@eborden
Copy link
Author

eborden commented Nov 22, 2017

Popping in here to see if you've had a chance to review.

@ndmitchell
Copy link
Owner

Alas, not yet - I've penciled in a train journey tomorrow morning to do a thorough review!

@eborden
Copy link
Author

eborden commented Jan 10, 2018

@ndmitchell How are you feeling about this initiative?

@ndmitchell
Copy link
Owner

Still positive, going through it slowly. Unfortunately other things cropped up so it's currently 5th on my 3rd todo list (I work through them all in parallel).

@ndmitchell
Copy link
Owner

Completed - thanks to your help and constant enthusiasm. https://hackage.haskell.org/package/filepattern is the result, and it's vastly better than the stuff in Shake.

@eborden
Copy link
Author

eborden commented Feb 13, 2019

Awesome!

@ndmitchell
Copy link
Owner

ndmitchell commented Feb 16, 2019

Plan is to wait until filepattern is used in HLint, ideally extensively (there are a few good places it could fit) and then we can sanity test it before turning it loose in Shake.

Once we do turn it lose the plan is make a release with everything but this. Then make a 0.1 bump with just the change to filepattern. Also requires removing // from the documentation.

@ndmitchell
Copy link
Owner

As it happened, for #642 I needed to use exactly the nice clean and optimised functions that were in filepattern, so I added the dependency. Most FilePattern values are the internal Shake implementation, but this particular one for --lint-watch is the new style. Given it's a debug only feature it shouldn't hurt too much.

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

No branches or pull requests

2 participants