GenEntryContent with parametrised content#102
Conversation
|
The direction looks good to me. Thanks, waiting until it becomes not a work-in-progress. |
|
This is ready for review, every piece is in place, but I would rather not spend time polishing at this point, before its agreed this is the way forward. |
Bodigrim
left a comment
Bodigrim
left a comment
There was a problem hiding this comment.
I'm content with the direction here and have only a few comments. To be sure, let's wait until the next weekend just in case someone suddenly comes up with a different, less invasive solution (or finds a fatal flaw in this approach).
phadej
force-pushed
the
genentrycontent-param
branch
from
ba666d2 to
3566aa1
Compare
phadej
force-pushed
the
genentrycontent-param
branch
from
3566aa1 to
aa8f734
Compare
Bodigrim
left a comment
Bodigrim
left a comment
There was a problem hiding this comment.
LGTM, thanks!
If there are no other comments or suggestions, I'll merge it on Sunday (feel free to remind me if I forget).
| pack' | ||
| :: FilePath | ||
| -> [FilePath] | ||
| -> IO [GenEntry OsPath TarPath LinkTarget] |
There was a problem hiding this comment.
Given that we are going for a major version bump anyway, maybe it's time to switch from FilePath to OsPath everywhere... Although one can argue that GenEntry OsPath is a detail implementation and should be semi-hidden behind a type synonym...
(Not asking you to change anything, just thinking aloud)
|
Thanks a lot! |
2 participants
See #100 (comment)