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

Use TH quotes more to eliminate some CPP #394

Merged
merged 7 commits into from
Dec 26, 2024

Conversation

TeofilC
Copy link
Contributor

@TeofilC TeofilC commented Jun 7, 2024

This eliminates some CPP by switching to using TH quotes.

Before submitting your PR, check that you've:

  • Bumped the version number.
  • Ran stylish-haskell and otherwise adhered to the style guide.

After submitting your PR:

  • Update the Changelog.md file with a link to your PR.
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts).

@TeofilC TeofilC marked this pull request as ready for review June 7, 2024 17:40
@TeofilC TeofilC force-pushed the wip/teo/less-cpp branch from efc2ba2 to f9bd127 Compare June 7, 2024 19:15
@parsonsmatt parsonsmatt self-requested a review August 12, 2024 19:39
@TeofilC
Copy link
Contributor Author

TeofilC commented Oct 30, 2024

What do you think about this @parsonsmatt ? If you are a fan of this direction, I can try to also get rid of the CPP caused by DoE

@TeofilC
Copy link
Contributor Author

TeofilC commented Oct 30, 2024

I'll investigate these failures soon

-- `where` clause.
[]
]
[d| sqlSelectColCount _ = sqlSelectColCount (Proxy @($(pure joinedTypes))) |]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Huh - there are two mentions of sqlSelectColCount. This is one. Is it posible that the [d| ... |] here is doing newName instead of using the name in scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right. This is quite odd behaviour (but also the type of issues I was hoping to find with this PR). I'll make a GHC ticket.

Strangely as welll
[d| $(varP 'foo) = ...|] is fine but as soon as I try to add arguments, it fails to parse, so I moved those into a lambda

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@parsonsmatt parsonsmatt added this to the 3.5.13.2 milestone Dec 26, 2024
Copy link
Collaborator

@parsonsmatt parsonsmatt left a comment

Choose a reason for hiding this comment

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

This looks much better, thank you!

@parsonsmatt parsonsmatt merged commit a14ec3a into bitemyapp:master Dec 26, 2024
@TeofilC
Copy link
Contributor Author

TeofilC commented Dec 26, 2024

Cheers! Happy holidays

@TeofilC TeofilC deleted the wip/teo/less-cpp branch December 27, 2024 00:20
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