Fix parsedField comment or code
ndmitchell opened this issue · 3 comments
Looking at
proto3-wire/src/Proto3/Wire/Decode.hs
Lines 288 to 293 in 43d8220
It says "To comply with the protobuf spec, if there are multiple fields with the same field number, this will always return the last one" and then returns the first one. That seems to be an optimisation from @gbaz. Was that intentional? If so, perhaps update the comment? If not, perhaps update the code? Our optimised version used lastMay
there.
If I recall, we keep the list in reversed order. As we approach Easter, it is appropriate that in this code, the last shall be first.
If that''s the case, we should document it on the RawField type, and update the comment to parsedField. Might be worth adding a test though, since changing it to be the last doesn't seem to break anything for me.
It was also suggested we move to a newtype, which would be even clearer in this regard...