Skip to content

lots of CPP and cleanup of 'Data.Monoid/Semigroup' stuff#1876

Merged
parsonsmatt merged 5 commits intoyesodweb:masterfrom
Vlix:avoid-warnings-with-CPP
Jun 16, 2025
Merged

lots of CPP and cleanup of 'Data.Monoid/Semigroup' stuff#1876
parsonsmatt merged 5 commits intoyesodweb:masterfrom
Vlix:avoid-warnings-with-CPP

Conversation

@Vlix
Copy link
Copy Markdown
Contributor

@Vlix Vlix commented Jun 8, 2025

Mostly found all the imports of 'Data.Monoid' and 'Data.Semigroup' and removed as much as I can without breaking it.
Used CPP for backwards compatibility where needed.

Thought about using CPP to avoid deprecation warning of 'parseMonad', but defining a helper function seemed way more straightforward.

@Vlix
Copy link
Copy Markdown
Contributor Author

Vlix commented Jun 8, 2025

Also a split-off from #1862

@Vlix
Copy link
Copy Markdown
Contributor Author

Vlix commented Jun 8, 2025

Failed tests don't seem to be because of yesod itself, but permissions on Windows 🤔

Vlix added 2 commits June 9, 2025 16:44
Mostly found all the imports of 'Data.Monoid' and 'Data.Semigroup'
and removed as much as I can without breaking it.
Used CPP for backwards compatibility where needed.

Thought about using CPP to avoid deprecation warning of 'parseMonad',
but defining a helper function seemed way more straightforward.
@Vlix Vlix force-pushed the avoid-warnings-with-CPP branch from c9d339d to 43d6514 Compare June 9, 2025 14:45
@Vlix
Copy link
Copy Markdown
Contributor Author

Vlix commented Jun 9, 2025

Rebased on new master and also noticed pragmas were overused, so at least wanted to remove CPP pragmas that were obviously superfluous.

... tests are failing because of stack, it seems :(
Will have to be rerun.

Comment on lines +10 to +12
#if !MIN_VERSION_base(4,11,0)
import Data.Semigroup ((<>))
#endif
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

base-4.11 corresponds to GHC 8.4. We currently have a base lower bound on 4.10 for GHC 8.2. 8.4 was released in March 2018, almost 7 years ago, well outside of any reasonable need to preserve support.

Honestly I'm in favor of bumping the base lower bound and deleting all this CPP.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd be totally fine with that as well.

It's not my call to make, but if you want to, I can make 4.11 / GHC 8.4 the new lowest bound and remove all CPP that was needed for base >= 4.10

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@parsonsmatt
Do you want me to update the cabal files and remove the unnecessary code?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, let's do it! 🎉

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@parsonsmatt
Should I also bump the (patch) version of every package I set to base >= 4.11? (And add to the Changelog?)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That would be proper, but I'd also hate to cause this PR to get bigger due to scope creep. We can always make a followup issue to drop GHC 8.4 support and do all related changes there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually doing the bumping and removing now makes this a less big PR (in terms of code changes), so I went ahead and did so.
If there are any further adjustments you need me to make, do tell 👍

@parsonsmatt
Copy link
Copy Markdown
Collaborator

Nice!

@Vlix
Copy link
Copy Markdown
Contributor Author

Vlix commented Jun 10, 2025

Maybe wait with publishing the new patch versions until the final PR is accepted and merged? Vlix#1
Then all the tweaks/cleanup will be finished 😄 (at least what I intended to do)

@parsonsmatt parsonsmatt merged commit 9be4699 into yesodweb:master Jun 16, 2025
25 of 26 checks passed
JBetz pushed a commit to JBetz/yesod that referenced this pull request Oct 13, 2025
* lots of CPP and cleanup of 'Data.Monoid/Semigroup' stuff

Mostly found all the imports of 'Data.Monoid' and 'Data.Semigroup'
and removed as much as I can without breaking it.
Used CPP for backwards compatibility where needed.

Thought about using CPP to avoid deprecation warning of 'parseMonad',
but defining a helper function seemed way more straightforward.

* yesod-core: found a better solution with 'defaultRequest'

* removed a bunch of pragmas that weren't used

* removal of Semigroup stuff outside of libraries

* bump to 'base >= 4.11' on libraries that had Semigroup imports (and remove the imports)
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.

2 participants