Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

ci: generate ts typings for grpc client stub #232

Merged
merged 8 commits into from
Jun 6, 2023

Conversation

sebastiendan
Copy link
Member

@sebastiendan sebastiendan commented May 12, 2023

Description

#229 introduced JavaScript gRPC client stubs generation through a new CI workflow. This PR adds the generation of TypeScript typings so that TypeScript clients can better use our client stubs.

Additionally, this PR changes the npm package publication flow to the following one:

  • First of all, the CI workflow will only run when
    • the push on main contains changes to protobuf files
    • the PR contains changes to protobuf files (coming from the active update or previous ones)
  • On PR updates, a step will check whether the npm package version was bumped and make the job fail if not
  • On merge on main, the npm publish step is run if the above was successful

Testing this new logic:

  • run that shows a failure because of missing version bump
  • run that shows a success because version was bumped

Additions and Changes

  • Add the grpc_tools_node_protoc_ts npm package as dep
  • Install protoc in the CI workflow
  • Use protoc with a bin packaged in grpc_tools_node_protoc_ts to generate typings
  • Bump npm package version to 0.1.1

PR Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added or updated tests that comprehensively prove my change is effective or that my feature works

@sebastiendan sebastiendan self-assigned this May 12, 2023
@sebastiendan sebastiendan force-pushed the ci/generate-grpc-ts-typings branch 2 times, most recently from 7a01321 to 0848663 Compare May 12, 2023 20:56
@sebastiendan sebastiendan force-pushed the ci/generate-grpc-ts-typings branch from 0848663 to 4c15782 Compare May 12, 2023 21:00
@sebastiendan sebastiendan marked this pull request as ready for review May 12, 2023 21:06
@sebastiendan sebastiendan requested a review from a team as a code owner May 12, 2023 21:06
@sebastiendan sebastiendan requested review from Freyskeyd and gruberb May 12, 2023 21:06
Copy link
Member

@Freyskeyd Freyskeyd left a comment

Choose a reason for hiding this comment

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

Could you rename the github action workflow to gRPC client stubs generation as we could add more language to this workflow at some point.

.github/workflows/grpc-client-stubs.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #232 (8b78e53) into main (50226f8) will decrease coverage by 0.32%.
The diff coverage is n/a.

❗ Current head 8b78e53 differs from pull request most recent head cb73631. Consider uploading reports for the commit cb73631 to get more accurate results

@@            Coverage Diff             @@
##             main     #232      +/-   ##
==========================================
- Coverage   59.85%   59.53%   -0.32%     
==========================================
  Files         171      170       -1     
  Lines        9752     9644     -108     
==========================================
- Hits         5837     5742      -95     
+ Misses       3915     3902      -13     

see 12 files with indirect coverage changes

@sebastiendan sebastiendan force-pushed the ci/generate-grpc-ts-typings branch from 0ea8843 to bc24941 Compare May 24, 2023 19:46
@sebastiendan sebastiendan force-pushed the ci/generate-grpc-ts-typings branch from 06efde5 to cb73631 Compare May 24, 2023 19:58
@sebastiendan sebastiendan requested review from Freyskeyd and gruberb May 24, 2023 20:06
Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the detailed write-up!

Just wondering if there is a scenario where the types of the JS gRPC client could be newer than the current release? Merging with main doesn't mean release, so the first logic seem to make more sense to me (because now we could have a situation with unreleased, but updated types in main the JS client is using, but the currently deployed topos network is not yet using).

Copy link
Contributor

@gruberb gruberb left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the detailed write-up!

Just wondering if there is a scenario where the types of the JS gRPC client could be newer than the current release? Merging with main doesn't mean release, so the first logic seem to make more sense to me (because now we could have a situation with unreleased, but updated types in main the JS client is using, but the currently deployed topos network is not yet using).

@sebastiendan
Copy link
Member Author

Looks good, thanks for the detailed write-up!

Just wondering if there is a scenario where the types of the JS gRPC client could be newer than the current release? Merging with main doesn't mean release, so the first logic seem to make more sense to me (because now we could have a situation with unreleased, but updated types in main the JS client is using, but the currently deployed topos network is not yet using).

Housekeeping: This was replied on Slack

@sebastiendan sebastiendan merged commit f4b292a into main Jun 6, 2023
@sebastiendan sebastiendan deleted the ci/generate-grpc-ts-typings branch June 6, 2023 20:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants