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

Add version flag and version info in generated code #1231

Merged
merged 9 commits into from
Apr 29, 2022
Merged

Add version flag and version info in generated code #1231

merged 9 commits into from
Apr 29, 2022

Conversation

meling
Copy link
Contributor

@meling meling commented Apr 28, 2022

Fixes #1216.

Example output (edited):

"use strict";
/**
 * @fileoverview gRPC-Web generated client stub for ag
 * @enhanceable
 * @public
 */
exports.__esModule = true;
exports.AutograderServiceClient = void 0;
// Code generated by protoc-gen-grpc-web. DO NOT EDIT.
// versions:
//  protoc-gen-grpc-web v1.3.1
//  protoc              v3.19.4
// source: ag/ag.proto
/* eslint-disable */
// @ts-nocheck

Caveat: Needs manual updating of newly added version.h file. The process of deciding the version number is not clear to me, and whether or not it could be automated somehow.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 28, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Collaborator

@sampajano sampajano left a comment

Choose a reason for hiding this comment

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

@meling Thanks so much for the change!! Much appreciated! :)

(It's totally ok that we have to manually update the version file for now.. probably the best thing that can be done for now :))

javascript/net/grpc/web/generator/version.h Outdated Show resolved Hide resolved
javascript/net/grpc/web/generator/grpc_generator.cc Outdated Show resolved Hide resolved
javascript/net/grpc/web/generator/grpc_generator.cc Outdated Show resolved Hide resolved
Copy link
Collaborator

@sampajano sampajano left a comment

Choose a reason for hiding this comment

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

Thanks so much again! LGTM just a few tiny nits and good to merge! :D

javascript/net/grpc/web/generator/grpc_generator.cc Outdated Show resolved Hide resolved
javascript/net/grpc/web/generator/grpc_generator.cc Outdated Show resolved Hide resolved
javascript/net/grpc/web/generator/grpc_generator.cc Outdated Show resolved Hide resolved
@meling
Copy link
Contributor Author

meling commented Apr 29, 2022

Note: I added "versioned" comments also for PrintMethodDescriptorFile and PrintGrpcWebClosureES6File for consistency.

PS: Feel free to squash the commits into one for the merge.

Copy link
Collaborator

@sampajano sampajano left a comment

Choose a reason for hiding this comment

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

Thanks so much for your contribution and your patience on the comments! Much appreciated! :)

@sampajano sampajano merged commit 1779661 into grpc:master Apr 29, 2022
@sampajano
Copy link
Collaborator

BTW i've build a few binaries (for all 3 platforms) here:
https://github.com/grpc/grpc-web/actions

In case anyone is interested in downloading them :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add version flag and version info in generated code
3 participants