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

Generate Paths_ module with qualified Data.List.last import #7650

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

alexbiehl
Copy link
Member


Please include the following checklist in your PR:

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

@Mikolaj
Copy link
Member

Mikolaj commented Sep 14, 2021

@alexbiehl: this is great for a quickfix, but what do you think about trying not to use Prelude at all and then adding a test that indeed turns off Prelude and compiles Paths_ so that don't stumble into this problem again?

Also, whatever happend to the previous PR? :)

@phadej
Copy link
Collaborator

phadej commented Sep 14, 2021

turning of prelude can be done with NoImplicitPrelude in the Paths_ file (when such extension is available), and doing import Prelude () otherwise.

@jneira
Copy link
Member

jneira commented Sep 14, 2021

After some problems to build cabal from this commit, i've tried it with policeman package and it worked 👍

@jneira
Copy link
Member

jneira commented Sep 14, 2021

This is the minimum change (so minimum risk to break things) which fixes it so imo we could merge this and do the definitive fix in another one on top of it

@alexbiehl
Copy link
Member Author

Thanks @jneira for trying it on the Kowainik packages!!

@Mikolaj
Copy link
Member

Mikolaj commented Sep 16, 2021

@alexbiehl: this is great for a quickfix, but what do you think about trying not to use Prelude at all and then adding a test that indeed turns off Prelude and compiles Paths_ so that don't stumble into this problem again?

Also, whatever happend to the previous PR? :)

@alexbiehl: ping? we should probably stick this into 3.6.2 RSN, so what's your recommendation about the wider context?

@alexbiehl
Copy link
Member Author

@Mikolaj My recommendation is to merge as is. Refactoring the code to avoid Prelude will take more time and I'd need to spend substantial time on it.

@Mikolaj
Copy link
Member

Mikolaj commented Sep 16, 2021

Got it. Let's do that.

@Mikolaj Mikolaj added attention: needs-review merge me Tell Mergify Bot to merge labels Sep 16, 2021
@Mikolaj
Copy link
Member

Mikolaj commented Sep 16, 2021

@Mergifyio backport 3.6

@mergify
Copy link
Contributor

mergify bot commented Sep 16, 2021

Command backport 3.6: pending

backport 3.6 is pending

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

lgtm. many thanks for the fix

@mergify
Copy link
Contributor

mergify bot commented Sep 17, 2021

Command backport 3.6: success

Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants