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

Add autogen-includes field #6037

Merged
merged 1 commit into from
May 8, 2019
Merged

Add autogen-includes field #6037

merged 1 commit into from
May 8, 2019

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented May 7, 2019

Fixes #5223

Doesn't #5169


Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

@phadej phadej requested a review from hvr May 7, 2019 10:15
@23Skidoo
Copy link
Member

23Skidoo commented May 7, 2019

WIP

Pro-tip: you can now use the Draft PR feature instead of this.

@phadej
Copy link
Collaborator Author

phadej commented May 7, 2019

Are CI performed on draft PRs? https://github.blog/2019-02-14-introducing-draft-pull-requests/ doesn't mention,

@ezyang
Copy link
Contributor

ezyang commented May 7, 2019 via email

@phadej phadej force-pushed the autogen-includes branch 2 times, most recently from 8800ec5 to 4dd2c29 Compare May 7, 2019 15:13
@phadej phadej changed the title WIP: autogen-includes Add autogen-includes field May 7, 2019
@phadej phadej force-pushed the autogen-includes branch 2 times, most recently from 25c7b63 to 428279b Compare May 7, 2019 15:15
@phadej
Copy link
Collaborator Author

phadej commented May 7, 2019

Ready for review

@hvr hvr added this to the 3.0 milestone May 7, 2019
Copy link
Member

@hvr hvr left a comment

Choose a reason for hiding this comment

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

lgtm (barring a minor grammatical issue)

@phadej phadej force-pushed the autogen-includes branch from 428279b to c8c3f94 Compare May 7, 2019 21:22
Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

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

LGTM modulo minor comments.

@phadej phadej force-pushed the autogen-includes branch 2 times, most recently from 8f31086 to 400a936 Compare May 8, 2019 04:17
autogen-includes aren't searched nor packages by `sdist`.

This is relatively small code patch, but there are
- change in file-format
- short documentation of the field
- `cabal check`
- test-suite noise due new field in `BuildInfo`
@phadej phadej force-pushed the autogen-includes branch from 400a936 to 7e27ae7 Compare May 8, 2019 08:15
@phadej phadej merged commit 93b0258 into master May 8, 2019
@phadej phadej deleted the autogen-includes branch May 8, 2019 13:34
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

Successfully merging this pull request may close these issues.

cabal new-build fails on time library
4 participants