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 GQL009 code fix formatting #3824

Merged
merged 1 commit into from Dec 15, 2023
Merged

Conversation

gao-artur
Copy link
Contributor

The formatting was broken when lambda's parameter or body was on a different line.

@gao-artur gao-artur added the analyzers Changes to analyzers label Dec 14, 2023
@gao-artur gao-artur added this to the 7.7 milestone Dec 14, 2023
@gao-artur gao-artur self-assigned this Dec 14, 2023
@gao-artur gao-artur changed the title Fix GQL009 fix formatting Fix GQL009 formatting Dec 14, 2023
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (61ca525) 84.47% compared to head (806b6fc) 84.48%.
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3824      +/-   ##
==========================================
+ Coverage   84.47%   84.48%   +0.01%     
==========================================
  Files         418      418              
  Lines       19202    19203       +1     
  Branches     3011     3011              
==========================================
+ Hits        16220    16223       +3     
+ Misses       2271     2270       -1     
+ Partials      711      710       -1     

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

@Shane32
Copy link
Member

Shane32 commented Dec 14, 2023

lmk if I should issue a release

@gao-artur
Copy link
Contributor Author

This was the only problem I found with our schema. Can you check on your schema to see if there are any formatting issues? If all good, I think we can release a fix.

@gao-artur gao-artur changed the title Fix GQL009 formatting Fix GQL009 code fix formatting Dec 14, 2023
@Shane32
Copy link
Member

Shane32 commented Dec 14, 2023

I had an issue where installing GraphQL 7.7.1 broke my CI script, with the following error:

The analyzer assembly '/home/runner/.nuget/packages/graphql.analyzers/7.7.1/analyzers/dotnet/cs/GraphQL.Analyzers.dll' references version '4.6.0.0' of the compiler, which is newer than the currently running version '4.3.0.0'.

However, changing actions/setup-dotnet@v1 to v4 solved the problem. (FYI this project uses .NET 6)

@Shane32
Copy link
Member

Shane32 commented Dec 14, 2023

Only formatting thing I ran into:

image

It really did a great job with everything, making changes to over 200 lines of code perfectly!

@gao-artur
Copy link
Contributor Author

Only formatting thing I ran into:

I think we have already talked about this problem. I'll try to take a look this weekend and see how difficult will be the fix.

@gao-artur
Copy link
Contributor Author

I had an issue where installing GraphQL 7.7.1 broke my CI script, with the following error:

The analyzer assembly '/home/runner/.nuget/packages/graphql.analyzers/7.7.1/analyzers/dotnet/cs/GraphQL.Analyzers.dll' references version '4.6.0.0' of the compiler, which is newer than the currently running version '4.3.0.0'.

However, changing actions/setup-dotnet@v1 to v4 solved the problem. (FYI this project uses .NET 6)

Unfortunately, I can't find any release notes for Microsoft.CodeAnalysis packages. Should I downgrade the package so it can be built with older SDKs? What is the minimal SDK version we want to support?

@Shane32
Copy link
Member

Shane32 commented Dec 14, 2023

I'm not sure. What are the consequences if you simply reference 4.3.0.0 ? If we can reference the older version with no consequence then I would do so. One of my projects uses ASP.NET Core 2.1 (on .NET Framework) because it runs on an operating system that is not supported by .NET Core 3.1 or newer. (Technically this combination is still supported by MS, at least until the OS goes out of support.) But other users may have other reasons for using older TFMs/SDKs.

@Shane32
Copy link
Member

Shane32 commented Dec 14, 2023

Note that I was able to upgrade to GraphQL.NET 7.7.1 in the above scenario just fine, perhaps because the CI script installs SDKs for 2.1, 3.1, 5.0, 6.0 and 7.0.

@gao-artur
Copy link
Contributor Author

I'll have to test it with older versions. I have no idea what changed between versions, as MS doesn't want to manage any release notes
dotnet/roslyn#7150

@gao-artur gao-artur merged commit 3ee59cf into master Dec 15, 2023
14 checks passed
@gao-artur gao-artur deleted the analyzers/fix-awaitable-formatting branch December 15, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzers Changes to analyzers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants