Open froozen opened 8 years ago
The name is really bad. Should we change it to API proposal
or something along those lines?
I’d like to add that the protobuf utils will be in the second level as well, since our rationale for not putting the protobuf things in the second level was that library users might want to heavily change the protobuf specifications, but the utils would stay the same.
I think both util functions in the second level should actually be in the fourth
one, since our custom Chan
type kind of belongs in there. selectChannel
would be bettor off as a prism, ie:
selectingChannel :: Word16 -> Prism' Packet Packet
Additionally, you didn’t apply all the arguments to Chan
– It would be Chan Packet Packet
instead of Chan Packet
. And I’d switch the arguments of
transform
in order to be parallel to fmap
.
I’d like to add that the [protobuf utils][1] will be in the second level as well, … I think both util functions in the second level should actually be in the fourth one, since our custom
Chan
type kind of belongs in there.
I don't think all code in the library has to be separated into the different levels. For me, it's just a way to create some sort of order in our implementation of the specs. Anything falling outside that, say utility functions and types, just exists without being bound to a certain level.
All the other suggestions
Agreed! I'll change those things right now.
What's @lukasepple's opinion on this so far?
Maybe the code on channels should be replaced by a link to the implementation #52?
The more parts of the proposal I implemented on lowest-two-levels
, the more the level seperation seems weird... What is meant to be in the fourth level? I don’t see anything new described in there compared to level two.
Shouldn't we just merge this?
Level two implements the parsing of raw ByteString
s into the Packet
type. This is something that probably won't change between protocol versions in the future.
Level four will implement the conversion of those Packet
s into more specific types, like ChatMessage
. As this is very dependant on protocol specification versions, it should reside in a higher level of the API.
Parsing protobuf messages is in level three though, so what’s new in level four?
This PR will serve as the thread for the discussion of the contents of this proposal.