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

Add public API baselines for rest of our implementation projects #24347

Closed
29 tasks done
dougbu opened this issue Jul 27, 2020 · 15 comments
Closed
29 tasks done

Add public API baselines for rest of our implementation projects #24347

dougbu opened this issue Jul 27, 2020 · 15 comments
Assignees
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Milestone

Comments

@dougbu
Copy link
Member

dougbu commented Jul 27, 2020

If joining me in this endeavour, please assign yourself and add your name in parentheses on the line or lines below. Would rather not have multiple people working on adding the same files in parallel 😺

I left MusicStore, ProjectTemplates, Shared, and Tools out because I'm pretty sure they don't contain $(IsImplementationProject) C# projects. If I'm wrong, please add to the list.

Mvc is already done.

@wtgodbe
Copy link
Member

wtgodbe commented Jul 27, 2020

Seems like maybe the kind of thing I could do on ops duty? (Tuesday & Wednesday this week)

@dougbu
Copy link
Member Author

dougbu commented Jul 27, 2020

The PublicApi.*.txt files aren’t really intended for manual intervention. To add them copy almost-empty files like src\Mvc\Mvc.Abstractions\src\PublicAPI.Shipped.txt into the directories you're adding baselines to e.g.

  1. cp .\src\Mvc\Mvc.Abstractions\src\PublicAPI.Shipped.txt .\src\Http\Http.Abstractions\src\PublicAPI.Shipped.txt
  2. cp .\src\Mvc\Mvc.Abstractions\src\PublicAPI.Shipped.txt .\src\Http\Http.Abstractions\src\PublicAPI.Unshipped.txt
  3. . .\activate.ps1
  4. .\src\Http\startvs.cmd
  5. F6 # or whatever your favourite build gesture is
  6. Click on a RS0016 error
  7. Right click in editor on underscored symbol or go straight to the “quick fix” icon to its left
    Fixer. Control-. also works.
  8. Choose “Add Blah to public API” / “Fix all occurrences in … Solution”
  9. Click Apply
  10. F6 # yes, again to see if the fixer missed anything or if other RS00xx errors show up (not uncommon)
  11. Suppress or fix other problems as needed but please suppress (if suppressing) using attributes and not globally or with #pragmas because attributes make the justification obvious

More information available in

@dougbu
Copy link
Member Author

dougbu commented Jul 27, 2020

@wtgodbe if you don't see anything to improve in the project templates tests and you have time, sure take something here. But, it's pretty much the last ops-on-call priority from my perspective.

@pranavkm pranavkm added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Jul 28, 2020
@dougbu dougbu added the Working label Aug 5, 2020
@Pilchie Pilchie self-assigned this Aug 28, 2020
@Pilchie
Copy link
Member

Pilchie commented Aug 28, 2020

Working on a few of these today.

@dougbu
Copy link
Member Author

dougbu commented Aug 28, 2020

Thanks @Pilchie

@dougbu
Copy link
Member Author

dougbu commented Aug 28, 2020

One note I forgot: src\Mvc\Mvc.Abstractions\src\PublicAPI.Shipped.txt contains the single line

#nullable enable

This may be incorrect in projects where #5680 hasn't been done. I'm not positive about this but recommend seeing if correct files are generated in such a project and, if necessary, starting with a completely empty file instead (where appropriate). Please let us know the results here😺

dougbu added a commit that referenced this issue Sep 5, 2020
- part of #24347

nits: take a few VS suggestions in opened files
- e.g. use pattern matching and `using` statements a bit more
@dougbu
Copy link
Member Author

dougbu commented Sep 5, 2020

src/Analyzers (@Pilchie) - files exist, but aren't used by the build yet.

Why would we enable the files to be used for analyzers❔ Would be simple to change but I'm not sure it's the Right Thing:tm: to do.

@Pilchie
Copy link
Member

Pilchie commented Sep 6, 2020

Ok, Components is up for grabs, but the rest are done. I can pick up Components if others want me to.

As for Analyzers, you're right. It's conceivable that people could reference them to build their own analyzers, but I'm not worried about preventing that.

@dougbu
Copy link
Member Author

dougbu commented Sep 6, 2020

I'll take Components. I started on it at one point because it was referenced in the *.slnf I used for HTML and HTTP at first. I'll finish that up very soon.

@dougbu
Copy link
Member Author

dougbu commented Sep 6, 2020

src/Analyzers (@Pilchie) - files exist, but aren't used by the build yet.

Should one of us remove those files❔

src/Framework (may contain no projects that need PublicApi..txt files) (@Pilchie)
src/Installers (may contain no projects that need PublicApi.
.txt files)

Confirmed src/Framework contains no C# code other than tests and src/Installers contains no *.csproj projects. Did you confirm the same thing @Pilchie

dougbu added a commit that referenced this issue Sep 6, 2020
- part of #24347

nits: take a few VS suggestions in opened files
- e.g. use pattern matching and `using` statements a bit more
@Pilchie
Copy link
Member

Pilchie commented Sep 6, 2020

Yes and Yes.

dougbu added a commit that referenced this issue Sep 6, 2020
- part of #24347
- unable to do src/Components/Authorization due to contained `*.razor` file
- ignored src/Components/Analyzers and src/Components/WebAssembly/Sdk
dougbu added a commit that referenced this issue Sep 6, 2020
dougbu added a commit that referenced this issue Sep 7, 2020
- part of #24347
- unable to do src/Components/Authorization due to contained `*.razor` file
- ignored src/Components/Analyzers and src/Components/WebAssembly/Sdk
@dougbu
Copy link
Member Author

dougbu commented Sep 8, 2020

Note to self: File an issue to follow up on a dotnet/roslyn#47350 fix to enable the analyzers by default for all implementation project except our analyzers, tools, msbuild tasks, and similar.

@Pilchie
Copy link
Member

Pilchie commented Sep 10, 2020

I think this is done now @dougbu

@dougbu
Copy link
Member Author

dougbu commented Sep 10, 2020

I agree and suggest you do the honours -- much of the work was your's❕ Many thanks

@Pilchie
Copy link
Member

Pilchie commented Sep 10, 2020

Fixed by a host of the linked PRs.

@Pilchie Pilchie closed this as completed Sep 10, 2020
@ghost ghost added Done This issue has been fixed and removed Working labels Sep 10, 2020
@Pilchie Pilchie added this to the 5.0.0-rc2 milestone Sep 10, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Oct 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework Done This issue has been fixed
Projects
None yet
Development

No branches or pull requests

4 participants