-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Initialize project (row-types version) #1
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
Conversation
Remove unsupported lts yamls
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.
You already know the feelings I have about the fancy haskell stuff here, but I'll try to reiterate it.
I think a simpler design is going to work out better:
data CloudEvent extension data = CloudEvent {
extension :: extension,
data :: data,
...
}
data KafkaExtension = KafkaExtension {
partitionKey :: Text
}
newtype KafkaCloudEvent data = KafkaCloudEvent (CloudEvent KafkaExtension data)The tradeoff of this design is that it does not get compiler errors when an extension's name conflicts with the CloudEvent top level properties. Given that a user of this library will most likely not be using CloudEvent directly I don't see this as too big of a downside. The most frequent usage of CloudEvent will be in library maintainers implementing new bindings but those are governed by a spec and the likelihood of a conflict seems non-existent.
| - FlexibleInstances | ||
| - GADTs | ||
| - GeneralizedNewtypeDeriving | ||
| - ImportQualifiedPost |
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.
If you follow our style guide and set the language to GHC2021, you don't need most of these extensions explicitly enabled.
| ceFocus | ||
| :: ( Functor f | ||
| , KnownSymbol l | ||
| , r .! l ≈ a | ||
| , r ~ Rec.Modify l a r' | ||
| , r' .! l ≈ b | ||
| , r' ~ Rec.Modify l b r | ||
| ) | ||
| => Label l -> (a -> f b) -> CloudEventRec r -> f (CloudEventRec r') | ||
| ceFocus l f r = | ||
| let | ||
| rec = getCloudEventRec r | ||
| updatedRec = Rec.focus l f rec | ||
| in | ||
| CloudEventRec <$> updatedRec |
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 haven't the slightest idea what's going on here. Does this function do something complicated?
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.
With great power of types comes great responsibility to write documentation and usage examples
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.
It's kinda like zoom from the curricula project
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.
Yeah, I should add more documentation. I kinda wanted to get the team's thoughts on the overall approach first
|
I suggest writing some tests and putting something into |
mjgpy3
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'm blocking to second AJ's suggestion.
The complexity of these new types far outweighs any benefits I can perceive. I'd also like to not have to learn a new DSL to construct an event.
|
I get your point @mjgpy3 (and @z0isch), while I do agree somewhat, I think the complexity here is pretty isolated in that a user never has to interface with row-types directly but only the Furthermore this much complexity isn't completely unheard of in the realm of SDKs given it's all for the sake of DevEx. Nevertheless, let me write up some example as @chris-martin suggests to show you what I mean |
|
@chris-martin added some more details and an example to the README.md |
| Support for release candidates is not required, but strongly encouraged. | ||
|
|
||
| \* Note: v1.0 is a special case and it is recommended that as long as v1.0 | ||
| is the latest version, SDKs should also support v0.3. |
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.
Restyled by prettier-markdown
| is the latest version, SDKs should also support v0.3. | |
| is the latest version, SDKs should also support v0.3. |
| `Integer` values in this range. | ||
| - String encoding: Integer component of the JSON Number per | ||
| [RFC 7159, Section 6](https://tools.ietf.org/html/rfc7159#section-6) | ||
| optionally prefixed with a minus sign. |
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.
Restyled by prettier-markdown
| optionally prefixed with a minus sign. | |
| optionally prefixed with a minus sign. |
|
Well that readme definitely helps make the project not look scary anymore. |
Unofficial Haskell SDK for the cloudevents spec. Written with row types to allow for arbitrary extension attributes as per the spec requirements. This gives us field deduplication at the type level but I fear I may have gone too far off the deep end just for that.
Anyways, this is a bare bones SDK that only allows for: