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

Update snippets to dotnet8 #41013

Merged
merged 10 commits into from
May 29, 2024
Merged

Conversation

rextor92
Copy link
Contributor

@rextor92 rextor92 commented May 20, 2024

Summary

Fixes #40684

Updated projects from .NET 6 & .NET 7 to .NET 8. Keeping in draft mode to review myself, but will leave a few questions.


  • Clean up unnecessary <LangVersion> tags
  • Unsure what to do with this project and its dockerfile

/samples folder is excluded from the versionsweeper.json, but I think it's a good idea to update samples there as well (unless it's a specific sample for older .NET version). Maybe a new task for that?


Internal previews

📄 File 🔗 Preview link
docs/standard/assembly/type-forwarding.md Type forwarding in the common language runtime

@rextor92
Copy link
Contributor Author

Hey @IEvangelist, I have some more work to do on this and review if I hadn't updated something by mistake, but I need some guidance with the snippets build task; namely

I suppose they were removed as they were not referenced anywhere. Should I backtrack and re-add them?

@IEvangelist
Copy link
Member

Hey @IEvangelist, I have some more work to do on this and review if I hadn't updated something by mistake, but I need some guidance with the snippets build task; namely

I suppose they were removed as they were not referenced anywhere. Should I backtrack and re-add them?

I wouldn't re-add them, no. Perhaps remove their projects, if those aren't referenced anymore either. Thanks for doing this, looks great thus far! I viewed EVERY SINGLE change 🤣.

@rextor92 rextor92 marked this pull request as ready for review May 21, 2024 12:25
@rextor92 rextor92 changed the title Update samples to dotnet8 Update snippets to dotnet8 May 21, 2024
@rextor92
Copy link
Contributor Author

I tried to remediate some Snippets 5000 issues in this commit which seems to have fixed the errors I had in the prior runs. Then I encountered a new boss: the schrödinger's solutions - they don't exist at the target location, yet they throw an error. Now I am in awe, have doubts about said commit and question my life choices.

Jokes aside, I am unsure how to proceed and am open to suggestions.

@adegeo
Copy link
Contributor

adegeo commented May 22, 2024

I'll investigate the snippets tool to see where that bug is. It seems it does flip out because of the deleted SLN, and not on anything actually valid.

One thing to do though, the security snippets folders were moved around and their current folder structure doesn't follow our guidelines. You have:

security\snippets\csharp\decrypting-data
security\snippets\vb\decrypting-data

Which is ...\snippets\language\article-name\code but the structure should be ...\snippets\article-name\language\code. You should have:

security\snippets\decrypting-data\csharp
security\snippets\decrypting-data\vb

adegeo
adegeo previously requested changes May 22, 2024
Copy link
Contributor

@adegeo adegeo left a comment

Choose a reason for hiding this comment

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

Snippets check should be fixed now. Once you add the commit to swap those folders around and then adjust the dependabot config, that will trigger a recheck. Thanks!

@adegeo
Copy link
Contributor

adegeo commented May 22, 2024

Looking...

@adegeo
Copy link
Contributor

adegeo commented May 22, 2024

Ahh OK, so I think those two solution files (readded via the revert) were added before we had perfected some of our detection. There should be 0 or 1 solution files in that folder. You can put all projects in the same solution and see if that works, but in the past I've seen it complain when the projects are named the same and only the language is different. Hopefully .NET/Visual Studio has solved that problem lol.

I would suggest deleting the solution files.

@adegeo
Copy link
Contributor

adegeo commented May 22, 2024

Snippets are clean!

Can you see the details of the OpenPublishing.Build action?

@rextor92
Copy link
Contributor Author

rextor92 commented May 22, 2024

That worked, thank you!
I deleted some files after this run - was throwing errors because of some unused snippets being removed here.

I can re-add all the necessary files to make it build, but if detecting unused snippets is automated it would delete files again in this specific case. Alternatively, I could inline the code referred in the after regions of type forwarding.

What would be the preferred approach?

@adegeo
Copy link
Contributor

adegeo commented May 22, 2024

Considering how small those "after" files are, and the code doesn't really demonstrate much, I would be fine with copying that code directly into the article.

@IEvangelist IEvangelist requested a review from adegeo May 29, 2024 23:25
@IEvangelist IEvangelist dismissed adegeo’s stale review May 29, 2024 23:26

Snippets 5000: Are green ✅

@IEvangelist IEvangelist merged commit fd5dfd0 into dotnet:main May 29, 2024
8 checks passed
@rextor92
Copy link
Contributor Author

Thanks for your help with this folks!

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.

Upgrade from net7.0 to LTS (or STS) version
4 participants