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

Option to ignore deprecated fields #29

Closed
wants to merge 46 commits into from

Conversation

jalandis
Copy link

Overview

Adds a new protobuf plugin option to ignore all deprecated fields + messages.

--elm_opt=remove-deprecated

While implementing the new feature I refactored a bit as I went. The number of changes snowballed a little lotta bit.

  • Switches to GO templating
  • Upgrades dependencies + switches to GO modules
  • Refactors using recommended GO package structure
  • Converts to Github Actions for test suite

Driving Use Case

Deprecating an API field requires the removal of all references from the frontend codebase. It would be nice to rely on compile-time errors to enforce this required deprecation.

Example protobuf with deprecated fields + messages.

message Bar {
    bool field = 1;
    bool deprecatedField = 2 [deprecated=true];
}
  
message Foo {
    option deprecated = true;

    bool field = 1;
}

Breaking Changes

The change to the supported GO version technically makes this a backward-incompatible change. Not sure if that is important for the project as it is a standalone executable. Just in case that does matter, it might be nice to include a few other breaking changes before any new release.

  1. Nested oneof values - the same solution for enums could be used (name collisions #8)
  2. Name collisions with multiple files (Incorrect code generated with type names #28)
  3. Optional decoders should fail with bad data (https://github.com/tiziano88/elm-protobuf/blob/master/tests/Main.elm#L43)
        ...
        , describe "better required"
            [ test "Decode required field with bad data" <| \() -> JD.decodeString (required "fail"  JD.int 0) badData |> Expect.err
            , test "Decode missing required field" <| \() -> JD.decodeString (required "succeed" JD.int 0) badData |> equal (Ok 0)
            ]
        ]


required : String -> JD.Decoder a -> a -> JD.Decoder a
required name decoder default =
    JD.maybe (JD.field name (JD.succeed Nothing))
        |> JD.andThen
            (\r ->
                case r of
                    Just _ ->
                        JD.field name decoder

                    Nothing ->
                        JD.succeed default
            )


badData : String
badData =
    """{ "fail": "wrong type" }"""

New decoder is similar to elm-json-decode-pipeline solution
https://github.com/NoRedInk/elm-json-decode-pipeline/blob/1.0.0/src/Json/Decode/Pipeline.elm#L111

jalandis and others added 30 commits December 22, 2020 16:14
… <= v < 8.0.0

Updates the requirements on [jweir/elm-iso8601](https://github.com/jweir/elm-iso8601) to permit the latest version.
- [Release notes](https://github.com/jweir/elm-iso8601/releases)
- [Changelog](https://github.com/jweir/elm-iso8601/blob/master/CHANGES.md)
- [Commits](jweir/elm-iso8601@5.0.0...7.0.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>
@tiziano88
Copy link
Owner

Wow, this is great, thanks for contributing! It will take me a while to review it, but I'll be happy to merge it as soon as I can!

@jalandis jalandis closed this Feb 15, 2024
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