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.

Sign up for GitHub

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

Jump to bottom

Add skipped failing test for IDE0044 #45288

Merged
merged 23 commits into from
Aug 31, 2020
Merged

Add skipped failing test for IDE0044 #45288

merged 23 commits into from
Aug 31, 2020

Conversation

Youssef1313
Copy link
Member

The linked issue is now closed and has Resolution-Fixed label. So, this false positive should have been fixed.

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 18, 2020 11:42
@mavasani
Copy link
Contributor

@Youssef1313 Seems like IDE0044 is still firing here.

@Youssef1313
Copy link
Member Author

Then the issue should be re-opened?
Pinging @sharwell as he worked on that issue.

@mavasani
Copy link
Contributor

@Youssef1313 The fix was made in the CodeStyle analyzer NuGet and we need to move to a newer CodeStyle package to get the fix.

@mavasani
Copy link
Contributor

https://github.com/dotnet/roslyn/blob/master/eng/Versions.props#L34

@Youssef1313
Copy link
Member Author

Aha, got it. Are there any reasons not to update?

@mavasani
Copy link
Contributor

Actually, the latest ones are here: https://dotnet.myget.org/feed/roslyn/package/nuget/Microsoft.CodeAnalysis.CSharp.CodeStyle

@Youssef1313 Youssef1313 requested a review from a team as a code owner June 18, 2020 15:36
@mavasani
Copy link
Contributor

@Youssef1313 eeba605 seems to be downgrading the version. Can you please upgrade to latest 3.8.x at https://dotnet.myget.org/feed/roslyn/package/nuget/Microsoft.CodeAnalysis.CSharp.CodeStyle version?

@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jun 19, 2020
@Youssef1313
Copy link
Member Author

@mavasani The failure is still there with the 3.8.x version. What could be the issue?

@sharwell
Copy link
Member

Updating the code style package is a bit complicated. The dotnet-format version needs to be updated at the same time to a version built from the same source, but the version numbers themselves will be different. I can check with @JoeRobich next week to see what the most recent matching pair is.

@Youssef1313
Copy link
Member Author

Using the latest dotnet-format version doesn't seem to work. I'll wait until you check the latest matching pair.

@Youssef1313
Copy link
Member Author

Ping @sharwell

@sharwell
Copy link
Member

@JoeRobich can you verify the matching versions here?

@JoeRobich
Copy link
Member

@JoeRobich can you verify the matching versions here?

Currently dotnet-format is tracking the VS 16.7 feed, so to generate a matching version I'll need to update to the 16.8 feed and trigger an update. It will pull in the most recent 3.8.0 build. I'll reply back when a build is ready with dotnet-format version and the M.CA version used.

@JoeRobich
Copy link
Member

@sharwell @Youssef1313 After updating dotnet-format to consume 3.8.0 builds, here is the matched pair.

Microsoft.CodeAnalysis - 3.8.0-1.20367.11
dotnet-format - 5.0.137002

@Youssef1313
Copy link
Member Author

@sharwell @JoeRobich The build is still failing.

@Youssef1313 Youssef1313 changed the title Remove IDE0044 suppression Fix IDE0055, Add skipped failing test for IDE0044 Aug 13, 2020
@Youssef1313
Copy link
Member Author

I reverted all version updates in this PR, and skipped the new failing test.
@mavasani, the issue can now be re-opened to address the skipped test.

@Youssef1313 Youssef1313 mentioned this pull request Aug 13, 2020
Closed
@Youssef1313 Youssef1313 changed the title Fix IDE0055, Add skipped failing test for IDE0044 Add skipped failing test for IDE0044 Aug 26, 2020
@Youssef1313 Youssef1313 marked this pull request as draft August 26, 2020 21:39
@Youssef1313 Youssef1313 marked this pull request as ready for review August 26, 2020 21:40
@Youssef1313
Copy link
Member Author

Please squash when merging.

@333fred
Copy link
Member

333fred commented Aug 27, 2020

@mavasani @sharwell what is this waiting on?

@Youssef1313 Youssef1313 marked this pull request as draft August 27, 2020 19:41
@Youssef1313 Youssef1313 marked this pull request as ready for review August 27, 2020 19:41
@sharwell
Copy link
Member

@333fred it should be ready to manually merge (with a squash commit per #45288 (comment)) when the build completes

@mavasani mavasani merged commit e7bad77 into dotnet:master Aug 31, 2020
@ghost ghost added this to the Next milestone Aug 31, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Aug 31, 2020
@333fred
Merge remote-tracking branch 'upstream/master' into warn-static-classes
8d069d3 
* upstream/master: (43 commits)
  Fix typos (dotnet#47264)
  Disable CS0660 where Full Solution Analysis produces a different result
  Pass in correct arguments to TypeScript handler
  Add skipped failing test for IDE0044 (dotnet#45288)
  Avoid first chance exception
  Code review feedback part 3
  Update stale URLs in the readme
  Update IntegrationTestSourceGenerator.cs
  Fix comment
  Update IntegrationTestSourceGenerator.cs
  Remove unused reference to obsolete class
  Update nullable annotations in Classification folder
  Update generator public API names: (dotnet#47222)
  NullableContextAttribute for property parameters is from accessor (dotnet#47223)
  Modify global analyzer config precedence: (dotnet#45871)
  Skip the C# source generator integration test running for now
  Additional XAML LSP handlers (dotnet#47217)
  Remap diagnostics using IWorkspaceVenusSpanMappingService in-proc
  Warn on type named "record" (dotnet#47094)
  Bail out early before getting SyntaxTree
  ...
@allisonchou allisonchou modified the milestones: Next, 16.8.P3 Aug 31, 2020
@sharwell sharwell added the Feature - IDE0044 Make field readonly label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeRobich JoeRobich JoeRobich approved these changes

@sharwell sharwell sharwell approved these changes

@333fred 333fred 333fred approved these changes

@mavasani mavasani mavasani approved these changes

Assignees

@mavasani mavasani

Labels
Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. Feature - IDE0044 Make field readonly
Projects
None yet
Milestone
16.8.P3
Development

Successfully merging this pull request may close these issues.
7 participants
@Youssef1313 @mavasani @sharwell @JoeRobich @333fred @jinujoseph @allisonchou