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

feat(proto): Pull version of protoc-gen-doc allowing proto3 optional #91

Merged
merged 5 commits into from
Jun 12, 2020
Merged

feat(proto): Pull version of protoc-gen-doc allowing proto3 optional #91

merged 5 commits into from
Jun 12, 2020

Conversation

ezimanyi
Copy link
Contributor

  • chore(proto): Bump protoc

  • style(proto): Break long lines in shell script

    This will make it easier to see what changed in the next commit
    (and any future changes we make here).

  • feat(proto): Add experimental flag

    As we've decided to use the new proto3 optional semantics to handle nullable fields, we need to pass this flag until the feature is fully GA.

  • feat(proto): Compile protoc-gen-doc instead of using release

    We'll need to temporarily use a fork of protoc-gen-doc so that we can include optional fields in proto3. As I don't want to actually make official releases of the fork, we'll want to build in the container from a specific tag/commit.

    This is what we used to do, before I changed it to pull the release
    (which is faster). This will be a bit slower the first time you run 'make proto' but should be temporary. Once the changes are in a released version upstream, we can go back to pulling an official release.

    If for some reason these changes don't end up upstream and it becomes an issue, I can make a release in my fork, but want to avoid that assuming this is temporary.

  • feat(proto): Pull version of protoc-gen-doc allowing proto3 optional

    This commit pulls in a version of protoc-gen-doc that allows proto3 optional protos. I've created a tag on my fork v1.3.2-proto3optional.1 corresponding to the changes I've submitted upstream. (The idea being that any other changes would be come v1.3.2-proto3optional.2, and if I rebase off of v1.3.3 it would become v1.3.3-proto3optional.1 but hopefully this will get merged quickly upstream so we won't need to update this much).

ezimanyi added 5 commits June 11, 2020 16:57
This will make it easier to see what changed in the next commit
(and any future changes we make here).
As we've decided to use the new proto3 optional semantics to
handle nullable fields, we need to pass this flag until the
feature is fully GA.
We'll need to temporarily use a fork of protoc-gen-doc so that
we can include optional fields in proto3. As I don't want to actually
make official releases of the fork, we'll want to build in the
container from a specific tag/commit.

This is what we used to do, before I changed it to pull the release
(which is faster). This will be a bit slower the first time you
run 'make proto' but should be temporary. Once the changes are in
a released version upstream, we can go back to pulling an official
release.

If for some reason these changes don't end up upstream and it becomes
an issue, I can make a release in my fork, but want to avoid that
assuming this is temporary.
This commit pulls in a version of protoc-gen-doc that allows proto3
optional protos. I've created a tag on my fork v1.3.2-proto3optional.1
corresponding to the changes I've submitted upstream. (The idea being
that any other changes would be come v1.3.2-proto3optional.2, and
if I rebase off of v1.3.3 it would become v1.3.3-proto3optional.1
but hopefully this will get merged quickly upstream so we won't need
to update this much).
@ezimanyi ezimanyi requested a review from maggieneterval June 11, 2020 20:57
Copy link
Contributor

@maggieneterval maggieneterval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔢 ❓ ➡️ 📝 👨‍💻 🍴

@maggieneterval maggieneterval merged commit 7721671 into spinnaker:master Jun 12, 2020
@ezimanyi ezimanyi deleted the optional-proto branch June 12, 2020 14:15
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