-
Notifications
You must be signed in to change notification settings - Fork 110
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
Make the bootstrapping process more robust: #66
Conversation
- Make a new package 'proto-lens-descriptors' for the descriptors - Name those modules "Proto.*" instead of "Bootstrap.Proto.*" - Add special logic so the descriptors modules don't use the reexported modules from proto-lens-protoc (otherwise we'd get a circular dependency) - Make the bootstrapping script use a special stack YAML file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this get proto-lens closer to being completely buildable by cabal?
@@ -0,0 +1,30 @@ | |||
Copyright Author name here (c) 2017 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -60,15 +61,21 @@ fileSyntaxType f = case f ^. syntax of | |||
"" -> Proto2 -- The proto compiler doesn't set syntax for proto2 files. | |||
s -> error $ "Unknown syntax type " ++ show s | |||
|
|||
-- Whether to import the "Reexport" modules or the originals; | |||
-- e.g., Data.ProtoLens.Reexport.Data.Map vs Data.Map. | |||
data UseReexport = UseReexport | UseOriginal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change this to [newtype around] ModuleName -> ImportDecl? UseOriginal would become importSimple
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to ImportDecl -> ImportDecl
. Since that type's pretty simple, I don't think it's worth adding a newtype.
|
@@ -71,11 +72,11 @@ data UseReexport = UseReexport | UseOriginal | |||
generateModule :: ModuleName | |||
-> [ModuleName] -- ^ The imported modules | |||
-> SyntaxType | |||
-> UseReexport | |||
-> (ImportDecl -> ImportDecl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A type synonym would still be nice for discoverability. You already found a name for it: ModifyImports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
- Make a new package 'proto-lens-descriptors' for the descriptors - Name those modules "Proto.*" instead of "Bootstrap.Proto.*" - Add special logic so the descriptors modules don't use the reexported modules from proto-lens-protoc (otherwise we'd get a circular dependency) - Make the bootstrapping script use a special stack YAML file.
modules from proto-lens-protoc (otherwise we'd get a circular dependency)