config samples (3) should adhere to NRT #36663
Conversation
embrace NRT
embrace NRT
Update GetConnectionStringByName
Update GetConnectionStringByProvider
ghost
added
the
community-contribution
There was a problem hiding this comment.
Hi @DickBaker
Overall this is good. I added one suggestion. In addition, can you add a default csproj file so that these samples are run through our CI build system.
After that's done, I'll 
...nippets_ADO.NET/DataWorks ConnectionStringSettings.RetrieveFromConfigByProvider/CS/source.cs
Show resolved
Hide resolved
|
Hi @DickBaker Can you make the suggested updates? |
|
sorry Bill I took my eye off this one (hoping that amateur [me] could leave it with the pros!). more significantly I thought that a large number of samples might be NRT-vintage, and that I might be opening a can of worms Substitute Pandora's box if you prefer that analogy and think my comment too rude your move Moriarty ! |
That's probably true. What we've been doing is similar to this PR: Whenever we update a sample, we enable NRT, and fix any warnings. That's why it helps to have the csproj files in place as well. (Historical note: Before moving to GitHub, the samples were stored in an internal system, and the migration tool only pulled the code that was displayed in an article, not the full sample that compiled. That's why some of the oldest samples don't have project files and fail the CI build) |
|
OK I'm game to give a punt to satisfying your if{} rules and create .csproj files so far having difficulty with ConnectionStringSettingsCollection so I will need to find assemblies to satisfy dependencies so your CI doesn't reject CSC/build fails. |
DataWorks DataSet.Merge has a DataSet.ReadXml overload warning but all projects should compile and comply with Bill's CI gates
|
Bill/David Hi ! As I suspected this ADO swamp was "alligators all the way down" but I persevered to crank out all missing *.csproj and bludgeon errors out of *.cs so compiles happily (under a VS .sln umbrella and minor editorconfig fiddle). I spent [far] too long pushing the water uphill, but I hope you will make good use to spuce up this unfashionable neighbourhood (ADO). Please let me know if/when you get to look at it [yes I know .NET-8 GA takes precedence!]. Cheers, Dick Output all items found, grouped by status... |
There was a problem hiding this comment.
Hi @DickBaker
Wow, you did a lot here.
The good news is that fixing the CI build will be pretty simple. I had suggestions for those.
I also reviewed the code changes, and all those look good, so once we have a clean CI build, I'll merge this.
Thanks again for all the work here! We really appreciate it! 🎆
| @@ -0,0 +1,74 @@ | |||
| # editorconfig.org | |||
There was a problem hiding this comment.
Let's remove this file from the PR.
Honestly, I think it mostly matches the global styles, but we'd rather not have to review multiple sources of truth. There's a global editorconfig in the root of the repo, and two others for the special case of turning on all code analysis rules in one area.
Otherwise, we expect all samples to follow the general guidelines.
|
Hi @DickBaker If you check the box that says "allow edits by maintainers", I'll make the final changes to this PR and merge it. |
Run snippets 5000 for PR https://github.com/dotnet/docs/actions/runs/6934430988/job/18862504635?pr=36663#step:6:1389 😭 Compile targets with unresolved issues: D:\a\docs\docs\samples\snippets\csharp\VS_Snippets_ADO.NET\DP LINQ to DataSet Examples\CS\LINQtoDataSetExamplesCS.csproj D:\a\docs\docs\samples\snippets\csharp\VS_Snippets_ADO.NET\DataWorks SqlClient.CAS\CS\Project.csproj D:\a\docs\docs\samples\snippets\csharp\VS_Snippets_ADO.NET\DataWorks SqlDependency.AspNet\CS\Project.csproj so I deleted those 3 projects [that I had unloaded locally in my VS, but files still in PR to break CI] - well, still contributing 50+ projects, so Bill/David can fix those 3 (or retire them) you probably still won't like DataWorks DataSet.Merge with CA5366 but you can nuke that too if bothersome - BTW "allow edits by maintainers" has always been checked Otherwise I'm happy with Bill's commits yesterday, so you have the wheel to drive this over the line!
Run dotnet/docs-tools/actions/status-checker@main so I've dropped that project too (leaving 4 to your senior magic) - hope this gives a clear round with remaining 53 projs
…iously unloaded so SYSLIB0003 coughed on 'SqlClientPermission' is obsolete: 'Code Access Security is not supported or honored by the runtime.' 'PermissionState' is obsolete: 'Code Access Security is not supported or honored by the runtime.' hence have nuked that project too [bummer] and trying again
|
I had trashed various projects that failed to build, i.e. files
so now of course the various *.md doc files (with refs to those hoary .cs files) now bleat "not found". Rats! I could reconstitute those .cs files for those .md files to reference however I think I have gone as far as I can, and passing the baton to Bill/David to decide whether to keep/drop docs to cr@p code
|
|
Closing in favor of #38401 There was a bit too much to do in the interactive editor, so I built a new PR on top of this one. |
|
yup, happy that YOU reinvented bunch of WinForms .NET4.8 etc stuff [that I'd mostly forgotten]. Seems that #38401 is still chewing on "Run dotnet/docs-tools/actions/status-checker@main" for 2hrs 41min as I write this, but hopefully will eventually resolve successfully to improve the ADO.NET story. Fair warning I'm looking at LINQ next ! |
I'm glad that you intend to continue helping us improve. My one request is to make more, smaller PRs. It will be a lot easier to review and merge them. Thanks. |
|
in case you haven't already spotted it the new [much simpler!] improvement is PR 38457 (successfully passed CI steps) |
VS shows squigglies on these 3x CS samples [and probably many others cf. #35172]