-
Notifications
You must be signed in to change notification settings - Fork 722
Remove todo for autogenerated modules #11390
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
base: master
Are you sure you want to change the base?
Remove todo for autogenerated modules #11390
Conversation
|
Thanks for the grammar checks @geekosaur. |
geekosaur
left a comment
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.
I should note that the original version of that warning isn't any better, so possibly some research is needed to clarify it.
| module files are not found if autogenerated files are not listed in | ||
| :pkg-field:`autogen-modules` fields. | ||
|
|
||
| :pkg-field:`executable:main-is` modules are not supported on |
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.
I'm finding this warning a little difficult to follow. Does it intend to say that autogen-modules can't be used with a custom main-is, or that the module in main-is can't be an autogenerated module, or maybe something else?
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.
I moved that warning from:
I've checked and it looks like there is not a cabal-testsuite test for the main-is warning (whichever way it goes).
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.
Right, I saw the original (see the comment above it, I forgot the actual review summary goes above the review comments). Maybe leave as is for now and open an issue specifically about that warning, since it would be good to say exactly what the problem is instead of something vague and possibly misleading. I've fixed a few such in GHC NOTEs but would need to study the code to see what's up here..
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.
I've updated the original warning turning it into a note and updated the main-is warning turning it into an error.
aa3ebee to
349154c
Compare
1ea10f1 to
a9bad34
Compare
- Review changes: - Prefer colon over semicolon - Need a comma between clauses - Need the definite article Co-Authored-By: brandon s allbery kf8nh <[email protected]>
9225ddb to
467c9ec
Compare
| appear twice for a component: once to flag them as being autogenerated in an | ||
| :pkg-field:`autogen-modules` field, and again to bring them into the component | ||
| with a listing in :pkg-field:`other-modules` or | ||
| :pkg-field:`library:exposed-modules`. |
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.
Is it worth noting why the second one is necessary (e.g. Paths_* is autogenerated but often — but not always — needs to be exported for library users)?
Fixes #11389, add documentation to the
autogen-modulesfield and edit the existing explanation and warning.