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

Correctly ignore fields using ObjectTypeExtension #4034

Merged
merged 6 commits into from
Aug 10, 2021

Conversation

tobias-tengler
Copy link
Collaborator

Closes #3776

@tobias-tengler tobias-tengler marked this pull request as ready for review August 1, 2021 12:03
@tobias-tengler tobias-tengler added the 👓 ready-for-review The PR is ready for review. label Aug 1, 2021
@tobias-tengler
Copy link
Collaborator Author

tobias-tengler commented Aug 1, 2021

@michaelstaib Do you see a better way to solve the issue at hand? It feels weird that I have to instantiate a ObjectFieldBinding in this case.

@tobias-tengler tobias-tengler changed the title WIP: Correctly ignore fields using ObjectTypeExtension Correctly ignore fields using ObjectTypeExtension Aug 1, 2021
@github-actions github-actions bot added the 📚 documentation This issue is about working on our documentation. label Aug 2, 2021
// if this definition is used for a type extension we need a
// binding to a field which shall be ignored. In case this is a
// definition for the type it will be ignored by the type initialization.
Definition.FieldIgnores.Add(new ObjectFieldBinding(
Copy link
Member

Choose a reason for hiding this comment

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

I have added a note why we have this binding here so that in case of a refactoring we know why this is done.

@michaelstaib michaelstaib added 🎬 ready Ready to merge and removed 👓 ready-for-review The PR is ready for review. labels Aug 10, 2021
foreach (ObjectFieldDescriptor field in Fields)
{
if (!field.Definition.Ignore)
{
Copy link
Member

Choose a reason for hiding this comment

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

I fixed the code style here, we always write the braces in C# code in order better see that

if(foo is true)
    dosomething();
    dosomething();   

is really

if(foo is true)
{
    dosomething();
}

dosomething();   

This issue quickly happens on refactorings and we decided to just enforce braces for if clauses.

@michaelstaib michaelstaib merged commit f85ddef into develop Aug 10, 2021
@michaelstaib michaelstaib deleted the tte/ignore-object-type-extension branch August 10, 2021 20:39
@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation This issue is about working on our documentation. 🌶️ hot chocolate 🎬 ready Ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignoring fields with ObjectTypeExtension<T> does not work
2 participants