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

refactor: re-write gen_gql_schema.rs #2297

Merged
merged 17 commits into from
Jul 8, 2024

Conversation

bnchi
Copy link
Contributor

@bnchi bnchi commented Jun 27, 2024

Summary:

  • Rewrite the module to create a ServiceDocument and use document::print to print it.
  • Drop all the write! calls from gen_gql_schema.rs
  • Add Derive macro to all configs.
  • Split the large file into smaller logical files that's easy to understand.
  • Make sure the final generated file doesn't change.
  • Add unit tests
  • Write documentation for different files and modules created.

note: the order in which the types show up in the file is a bit different from the old one, but I re-ordered them manually and computed the SHA of the old and the new file to see if there's any difference in the content, and they both gave the same hash.

Issue Reference(s):
Fixes #1510
/claim #1510

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>

Copy link

algora-pbc bot commented Jun 27, 2024

💵 To receive payouts, sign up on Algora, link your Github account and connect with Stripe/Alipay.

@github-actions github-actions bot added the type: chore Routine tasks like conversions, reorganization, and maintenance work. label Jun 27, 2024
@bnchi bnchi marked this pull request as ready for review July 3, 2024 10:09
@bnchi
Copy link
Contributor Author

bnchi commented Jul 3, 2024

@tusharmath ready for review

@tusharmath tusharmath self-assigned this Jul 3, 2024
@tusharmath
Copy link
Contributor

Is there a way to ensure nothing changed semantically between the previous and the current .tailcallrc.graphql

src/core/document.rs Outdated Show resolved Hide resolved
src/core/document.rs Outdated Show resolved Hide resolved
@tusharmath
Copy link
Contributor

There are only a few tiny comments on this PR. Overall it looks awesome and is a massive improvement over our current approach 🙌

Moving to draft to reduce noise and improve CI efficiency. Once you are ready just mark it as "ready to review". Feel free to give a shoutout on the #contributors channel on Discord if you want immediate attention.

@tusharmath tusharmath marked this pull request as draft July 5, 2024 10:44
@bnchi
Copy link
Contributor Author

bnchi commented Jul 6, 2024

Is there a way to ensure nothing changed semantically between the previous and the current .tailcallrc.graphql

The order of some types are a bit different what I did is I re-ordered the types in the generated file manually to match the old one it took a couple of minutes, removed all the white spaces and new lines from both files and computed the hash, they both gave the same hash means they both got the same results. If you would to do the same you should get these results.

MD5 (.tailcallrc.graphql) = 3e6bdc25a5f19820b72ad4ef7e80f4a3
MD5 (.old.graphql) = 3e6bdc25a5f19820b72ad4ef7e80f4a3

note there was a slight issue in the previous generated file due to :
#2297 (comment)

so you will have to update that in the old generated file to get the same hash

Copy link

codecov bot commented Jul 6, 2024

Codecov Report

Attention: Patch coverage is 66.36528% with 186 lines in your changes missing coverage. Please review.

Project coverage is 87.41%. Comparing base (e7c4fc7) to head (75987b3).
Report is 2 commits behind head on main.

Files Patch % Lines
src/core/config/config.rs 13.18% 79 Missing ⚠️
tailcall-typedefs-common/src/input_definition.rs 72.44% 35 Missing ⚠️
tailcall-typedefs/src/main.rs 0.00% 17 Missing ⚠️
...ilcall-typedefs-common/src/directive_definition.rs 82.75% 10 Missing ⚠️
tailcall-typedefs-common/src/lib.rs 76.66% 7 Missing ⚠️
src/core/document.rs 92.77% 6 Missing ⚠️
src/core/config/telemetry.rs 25.00% 3 Missing ⚠️
tailcall-typedefs-common/src/common.rs 91.42% 3 Missing ⚠️
src/core/config/link.rs 33.33% 2 Missing ⚠️
src/core/config/server.rs 50.00% 2 Missing ⚠️
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2297      +/-   ##
==========================================
+ Coverage   86.02%   87.41%   +1.38%     
==========================================
  Files         234      240       +6     
  Lines       22405    22448      +43     
==========================================
+ Hits        19274    19622     +348     
+ Misses       3131     2826     -305     

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

@bnchi bnchi marked this pull request as ready for review July 6, 2024 13:39
@bnchi bnchi requested a review from tusharmath July 6, 2024 13:39
@tusharmath tusharmath merged commit 719eddc into tailcallhq:main Jul 8, 2024
31 of 32 checks passed
@harshbisle
Copy link

Well done! @bnchi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Bounty claim type: chore Routine tasks like conversions, reorganization, and maintenance work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: re-write gen_gql_schema.rs
3 participants