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

WIP: Add Option to Drop Deprecated Messages and Fields #3

Closed
wants to merge 46 commits into from

Conversation

jalandis
Copy link

@jalandis jalandis commented Jan 30, 2021

Overview

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

--elm_opt=remove-deprecated

Other changes:

  • Switches to GO templating
  • Upgrades dependencies + switches to GO modules
  • Refactors using recommended GO package structure]

Driving Use Case

Deprecating an API field requires the removal of all references from the frontend code base. 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;
}

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]>
@jalandis jalandis changed the title Deprecated spike Add Option to Drop Deprecated Messages and Fields Jan 30, 2021
@jalandis jalandis changed the title Add Option to Drop Deprecated Messages and Fields WIP: Add Option to Drop Deprecated Messages and Fields Jan 30, 2021
@jalandis
Copy link
Author

@thematthopkins @jonpitch
I have prepped this Test PR with some ELM PB plugin changes I have been working on in my free time. It's in a pretty good place right now and I was hoping to get your feedback on the new feature and design changes. Of course, no pressure if you're busy or uninterested.

Interested in any feedback you have but a few specific items I have been noodling:

  • Any improvements to the type definitions
  • Any recommendations for the new GO elm package design (better encapsulation, etc)
  • Any recommendations on the project file structure
  • Any recommendations on the build scripts

Depending on feedback, I will prep a real PR for the main project and maybe pull it into our build system.

@jalandis
Copy link
Author

jalandis commented Feb 1, 2021

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 tiziano88/elm-protobuf#8)
  2. Name collisions with multiple files (Incorrect code generated with type names tiziano88/elm-protobuf#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 jalandis closed this Apr 2, 2021
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.

1 participant