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

fix: disable default inclusion of @tag directive #6630

Closed

Conversation

dariuszkuc
Copy link
Contributor

While useful, @tag directive is not part of the built-in directives so shouldn't be included by default. Users should opt-in to use custom directives.

While useful, `@tag` directive is not part of the built-in directives so shouldn't be included by default. Users should opt-in to use custom directives.
@CLAassistant
Copy link

CLAassistant commented Oct 25, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

HotChocolate.Data.Spatial.Filters.QueryableFilterVisitorTouchesTests.Create_Touches_Query [FAIL]

@github-actions
Copy link

StrawberryShake.CodeGeneration.CSharp.Integration.StarWarsUnionList.StarWarsUnionListTest.Execute_StarWarsUnionList_Test [FAIL]
StrawberryShake.CodeGeneration.CSharp.Integration.AnyScalarDefaultSerialization.AnyScalarDefaultSerializationTest.Execute_AnyScalarDefaultSerialization_Test [FAIL]
StrawberryShake.CodeGeneration.CSharp.Integration.UploadScalar.UploadScalarTest.Execute_InputNested_Argument [FAIL]
StrawberryShake.CodeGeneration.CSharp.Integration.StarWarsTypeNameOnUnions.StarWarsTypeNameOnUnionsTest.Execute_StarWarsTypeNameOnUnions_Test [FAIL]
StrawberryShake.CodeGeneration.CSharp.Integration.StarWarsGetHero.StarWarsGetHeroTest.Watch_CacheFirst_StarWarsGetHero_Test [FAIL]

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 90 lines in your changes are missing coverage. Please review.

Comparison is base (6965960) 78.98% compared to head (a5c44dc) 78.31%.
Report is 44 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6630      +/-   ##
==========================================
- Coverage   78.98%   78.31%   -0.67%     
==========================================
  Files        2903     2949      +46     
  Lines      139771   142272    +2501     
==========================================
+ Hits       110397   111425    +1028     
- Misses      29374    30847    +1473     
Flag Coverage Δ
unittests 78.31% <87.35%> (-0.67%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...e/src/CookieCrumble/Extensions/WriterExtensions.cs 100.00% <ø> (ø)
...CookieCrumble/src/CookieCrumble/TestEnvironment.cs 0.00% <ø> (ø)
...polloFederationRequestExecutorBuilderExtensions.cs 66.66% <100.00%> (ø)
...ensions/ApolloFederationSchemaBuilderExtensions.cs 86.66% <ø> (ø)
.../src/ApolloFederation/FederationTypeInterceptor.cs 100.00% <100.00%> (ø)
...ion/src/ApolloFederation/Helpers/ArgumentParser.cs 68.65% <100.00%> (+0.47%) ⬆️
...rc/AspNetCore.CommandLine/Command/ExportCommand.cs 100.00% <100.00%> (ø)
...rc/AspNetCore/Extensions/HttpResponseExtensions.cs 26.31% <ø> (+4.57%) ⬆️
...olate/AspNetCore/src/AspNetCore/HeaderUtilities.cs 80.85% <ø> (ø)
...ate/AspNetCore/src/AspNetCore/HttpGetMiddleware.cs 82.31% <ø> (ø)
... and 90 more

... and 346 files with indirect coverage changes

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

@michaelstaib
Copy link
Member

We include tag by default ... this is intended ... when the apollo integration is used it is the apollo federation tag version, so it should not be an issue.

@dariuszkuc dariuszkuc deleted the disableDefaultTagOption branch October 26, 2023 14:31
@dariuszkuc
Copy link
Contributor Author

Thanks for confirming. Wasn't sure if it was intentional as it looked like a miss (all other options are disabled by default and it is not a standard GraphQL feature) and our integration was somewhat iffy because of that (apollographql/federation-hotchocolate#36).

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.

3 participants