fix(grpc): remove package from type names #1747

shashitnak · 2024-04-17T10:36:47Z

Build & Testing:

I ran cargo test successfully.
I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

I have added relevant unit & integration tests.
I have updated the documentation accordingly.
I have performed a self-review of my code.
PR follows the naming convention of <type>(<optional scope>): <title>

Summary by CodeRabbit

Refactor
- Enhanced the handling of package data structures to improve performance and flexibility.

coderabbitai · 2024-04-17T10:36:54Z

Walkthrough

The recent update involves enhancing the GraphQLType struct with methods for modifying its package field and refining how packages are handled across the system. Specifically, methods for prepending and appending to the package vector have been added. Additionally, the handling of the package attribute has been standardized to use a vector of strings throughout, replacing any previous implementations that used a singular optional string.

Changes

File	Changes
`src/generator/.../type.rs`	Added `prepend_package()` and `append_package()` methods; refactored package handling to use vectors.
`src/generator/.../unparsed.rs`	Updated to store package as a vector of strings.
`src/generator/.../package.rs`	Refactored to handle paths strictly as a vector of strings.

Recent Review Details

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 9c57f92 and 1423084.

Files selected for processing (1)

src/generator/graphql_type.rs (4 hunks)

Files skipped from review as they are similar to previous changes (1)

src/generator/graphql_type.rs

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

coderabbitai · 2024-04-17T10:41:13Z

src/generator/from_proto.rs

+            let type_name = GraphQLType::new(enum_name)
+                .package(&self.package)
+                .as_enum()
+                .unwrap()
+                .to_string();


Consider handling the potential None case in the chain of method calls for type_name instead of using unwrap(), which could lead to runtime panics if the conversion fails.

- .unwrap() + .ok_or_else(|| "Failed to convert to enum".to_string())?

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

let type_name = GraphQLType::new(enum_name)

.package(&self.package)

.as_enum()

.unwrap()

.to_string();

let type_name = GraphQLType::new(enum_name)

.package(&self.package)

.as_enum()

.ok_or_else(|| "Failed to convert to enum".to_string())?

.to_string();

actually, the GraphQLType<Unparsed>::parse that is used by as_enum could be rewritten without return Option that could simplify usage of it and remove unwraps

@meskill, your suggestion to rewrite the GraphQLType<Unparsed>::parse method used by as_enum to not return an Option is intriguing. Removing the Option could indeed simplify the usage by eliminating the need for unwraps. However, this change would require a careful review of all places where this method is used to ensure that there are no cases where a None return is expected or required.

Would you like to proceed with exploring this change in the current PR, or would you prefer to handle this as a separate task? If it's the latter, I can help by opening a GitHub issue to track this enhancement. Please let me know how you'd like to proceed!

codecov · 2024-04-17T10:41:33Z

Codecov Report

Attention: Patch coverage is 87.87879% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 87.19%. Comparing base (b5fa454) to head (1423084).
Report is 61 commits behind head on main.

Files	Patch %	Lines
src/generator/graphql_type.rs	81.81%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1747      +/-   ##
==========================================
+ Coverage   86.93%   87.19%   +0.25%     
==========================================
  Files         155      155              
  Lines       15540    15841     +301     
==========================================
+ Hits        13510    13812     +302     
+ Misses       2030     2029       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/generator/snapshots/tailcall__generator__from_proto__test__from_proto.snap

coderabbitai

Actionable comments posted: 1

src/generator/graphql_type.rs

tusharmath · 2024-04-18T12:09:32Z

src/generator/snapshots/tailcall__generator__from_proto__test__from_proto.snap

+enum NEWS_STATUS @tag(id: "news.Status") {
+  DRAFT
+  PUBLISHED
+}


Add tests for messages inside of messages.

news.proto already contains message inside of message https://github.com/tailcallhq/tailcall/pull/1747/files#diff-771adfaaae91c7816aaf69af02c57749bfaaaa74e3db399e055104a8a1a6de23R26-R33

is it enough?

meskill · 2024-04-19T12:09:17Z

I'm not sure the name of the PR properly reflects the changes.

github-actions · 2024-04-21T13:03:16Z

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

github-actions · 2024-04-27T05:02:40Z

PR closed after 5 days of inactivity.

github-actions · 2024-05-01T12:03:53Z

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

github-actions · 2024-05-06T16:02:40Z

PR closed after 5 days of inactivity.

shashitnak added 3 commits April 16, 2024 14:43

minor grpc fixes

5649345

reverting conversion to snake case during validation

0bd007b

fixing snapshots

16a6ba7

github-actions bot added the type: fix Iterations on existing features or infrastructure. label Apr 17, 2024

coderabbitai bot reviewed Apr 17, 2024

View reviewed changes

fixing package prefix bug

88b6908

tusharmath reviewed Apr 18, 2024

View reviewed changes

src/generator/snapshots/tailcall__generator__from_proto__test__from_proto.snap Outdated Show resolved Hide resolved

add append and prepend APIs

e3a4373

coderabbitai bot reviewed Apr 18, 2024

View reviewed changes

src/generator/graphql_type.rs Outdated Show resolved Hide resolved

tusharmath reviewed Apr 18, 2024

View reviewed changes

src/generator/graphql_type.rs Outdated Show resolved Hide resolved

shashitnak added 2 commits April 18, 2024 11:42

fixing grpc method names

25ca494

fixing enum tag id

9c57f92

tusharmath reviewed Apr 18, 2024

View reviewed changes

src/generator/graphql_type.rs Outdated Show resolved Hide resolved

tusharmath reviewed Apr 18, 2024

View reviewed changes

tusharmath changed the title ~~fix(grpc): some small fixes to be able to run the generated tailcall Config from proto files~~ fix(grpc): remove package from type names Apr 18, 2024

tusharmath enabled auto-merge (squash) April 18, 2024 15:06

remove unused API

1423084

tusharmath approved these changes Apr 18, 2024

View reviewed changes

github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Apr 21, 2024

github-actions bot closed this Apr 27, 2024

auto-merge was automatically disabled April 27, 2024 05:02
Pull request was closed

meskill added this to the Generate GraphQL config from protobuf definitions milestone Apr 29, 2024

meskill reopened this Apr 29, 2024

github-actions bot removed the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Apr 29, 2024

github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label May 1, 2024

github-actions bot closed this May 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(grpc): remove package from type names #1747

fix(grpc): remove package from type names #1747

shashitnak commented Apr 17, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 17, 2024 •

edited

Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot Apr 17, 2024 •

edited

Loading

meskill Apr 19, 2024

coderabbitai bot Apr 19, 2024

codecov bot commented Apr 17, 2024 •

edited

Loading

coderabbitai bot left a comment

tusharmath Apr 18, 2024

meskill Apr 19, 2024

meskill commented Apr 19, 2024

github-actions bot commented Apr 21, 2024

github-actions bot commented Apr 27, 2024

github-actions bot commented May 1, 2024

github-actions bot commented May 6, 2024

fix(grpc): remove package from type names #1747

fix(grpc): remove package from type names #1747

Conversation

shashitnak commented Apr 17, 2024 • edited by coderabbitai bot Loading

Summary by CodeRabbit

coderabbitai bot commented Apr 17, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

meskill Apr 19, 2024

Choose a reason for hiding this comment

coderabbitai bot Apr 19, 2024

Choose a reason for hiding this comment

codecov bot commented Apr 17, 2024 • edited Loading

Codecov Report

coderabbitai bot left a comment

Choose a reason for hiding this comment

tusharmath Apr 18, 2024

Choose a reason for hiding this comment

meskill Apr 19, 2024

Choose a reason for hiding this comment

meskill commented Apr 19, 2024

github-actions bot commented Apr 21, 2024

github-actions bot commented Apr 27, 2024

github-actions bot commented May 1, 2024

github-actions bot commented May 6, 2024

shashitnak commented Apr 17, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Apr 17, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

coderabbitai bot Apr 17, 2024 •

edited

Loading

codecov bot commented Apr 17, 2024 •

edited

Loading