Skip to content

Clean up TODOs and duplicated code in client code generation #4923

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
Closed
3 tasks done
dougbu opened this issue Oct 7, 2018 · 5 comments
Closed
3 tasks done

Clean up TODOs and duplicated code in client code generation #4923

dougbu opened this issue Oct 7, 2018 · 5 comments
Assignees
@dougbu
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed task
Milestone
3.0.0-preview9

Comments

@dougbu
Copy link
Contributor

dougbu commented Oct 7, 2018 •
edited
Loading

Is this a Bug or Feature request?

Enhancement that's part of #4896

Description of the problem

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All

See the features/client.code.generation branch. That work is intended for the 2.2 Preview 3 milestone.
@dougbu dougbu self-assigned this Oct 7, 2018
@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 14, 2018
@aspnet-hello aspnet-hello assigned dougbu and unassigned dougbu Dec 14, 2018
@aspnet-hello aspnet-hello added this to the ClientCodeGen milestone Dec 14, 2018
@aspnet-hello aspnet-hello added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates task labels Dec 14, 2018
@mkArtakMSFT mkArtakMSFT modified the milestones: ClientCodeGen, 3.0.0-preview5 Feb 6, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.0.0-preview6, Backlog May 30, 2019
@dougbu dougbu mentioned this issue Jun 1, 2019
Merged
@dougbu
Copy link
Contributor Author

dougbu commented Jun 1, 2019 •
edited
Loading

New bullets from #10641 discussions:

This was referenced Jun 1, 2019
Merged
Merged
@dougbu
Copy link
Contributor Author

dougbu commented Jun 3, 2019 •
edited
Loading

New bullet from a #10669 discussion:

dougbu added a commit that referenced this issue Jun 3, 2019
@dougbu
Rewrite dotnet-getdocument and GetDocument.Insider to retrieve all do…
4c8ca0b 
…cuments in one go (#10667)

- #8242 1 of 2
- save a cache file listing all retrieved documents
- remove .NET Core App 2.0 support
- remove ServiceProjectReferenceMetadata.targets, related `Project` class, and searches for a project
  - tools will run within a project and get needed information from project on command line
- roll framework forward in both tools to expand their applicability when using .NET Core
- use Microsoft.Extensions.HostFactoryResolver.Sources (part of #4923)
  - remove Microsoft.AspNetCore.Hosting.Abstractions dependency

nits:
- refactor methods in `GetDocumentCommandWorker`
- reorder option addition for consistency and to place `--help` at the top of help output
- consolidate information about method signatures at top of `GetDocumentCommandWorker`
- consolidate `try` / `catch` blocks in `GetDocumentCommandWorker`
- shorten the lifespan of a `Task`
- ensure GetDocument.Insider exit codes are unique
- make a few more `string`s `const`
- fold a few expressions over fewer lines
@dougbu
Copy link
Contributor Author

dougbu commented Jun 11, 2019 •
edited
Loading

Couple more bullets

  • avoid NU1704 build warnings and other client / server incompatibility issues (4e4ccdd)
  • change default $(OpenApiGenerateDocuments) to false in projects using .NET Core 2.0 or earlier (4e4ccdd)
    • not supported by dotnet-getdocument tool in the Microsoft.Extensions.ApiDescription.Server package
    • otherwise, users need to manually add <OpenApiGenerateDocuments>false</OpenApiGenerateDocuments> when updating the NSwag or Swashbuckle package version to one that pulls in Microsoft.Extensions.ApiDescription.Server

@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, 3.0.0-preview8 Jun 18, 2019
@dougbu dougbu mentioned this issue Jun 19, 2019
Closed
3 tasks
@mkArtakMSFT mkArtakMSFT removed this from the 3.0.0-preview8 milestone Jul 18, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview9 milestone Jul 18, 2019
@dougbu dougbu added Working and removed 1 - Ready labels Aug 16, 2019
@dougbu
Copy link
Contributor Author

dougbu commented Aug 17, 2019

Part 1 in 04b1adb

dougbu added a commit to dotnet/extensions that referenced this issue Aug 21, 2019
@dougbu
Add switch to enable expected and unexpected arguments in any order
5afe994 
- unblocks work on dotnet/aspnetcore#4923
  - arguments for inside and outside men of service reference doc gen tool are mixed by default
  - users may add either argument type to the end of the outside man's command line
- e.g. "command --unexpected unexpectedValue --expected" can now set the "expected" option
  - only "--unexpected" and "unexpectedValue" are added to RemainingArguments in that case

- default behaviour of the command-line parser is unchanged to avoid breaking existing applications
- new switch is supported only when calling `CommandLineApplication` constructor for top-level commands
  - `dotnet-getdocument` (the outside man) has no (sub)commands and expanding scope would unnecessarily increase churn

nits: take VS suggestions in changed files
dougbu added a commit to dotnet/extensions that referenced this issue Aug 21, 2019
@dougbu
Add switch to enable expected and unexpected arguments in any order
c3d16c4 
- unblocks work on dotnet/aspnetcore#4923
  - arguments for inside and outside men of service reference doc gen tool are mixed by default
  - users may add either argument type to the end of the outside man's command line
- e.g. "command --unexpected unexpectedValue --expected" can now set the "expected" option
  - only "--unexpected" and "unexpectedValue" are added to RemainingArguments in that case

- default behaviour of the command-line parser is unchanged to avoid breaking existing applications
- new switch is supported only when calling `CommandLineApplication` constructor for top-level commands
  - `dotnet-getdocument` (the outside man) has no (sub)commands and expanding scope would increase churn

nits: take VS suggestions in changed files
@dougbu dougbu mentioned this issue Aug 21, 2019
Merged
dougbu added a commit to dotnet/extensions that referenced this issue Aug 22, 2019
@dougbu
Add switch to enable expected and unexpected arguments in any order
dc31b40 
- unblocks work on dotnet/aspnetcore#4923
  - arguments for inside and outside men of service reference doc gen tool are mixed by default
  - users may add either argument type to the end of the outside man's command line
- e.g. "command --unexpected unexpectedValue --expected" can now set the "expected" option
  - only "--unexpected" and "unexpectedValue" are added to RemainingArguments in that case

- default behaviour of the command-line parser is unchanged to avoid breaking existing applications
- new switch is supported only when calling `CommandLineApplication` constructor for top-level commands
  - `dotnet-getdocument` (the outside man) has no (sub)commands and expanding scope would increase churn

nits: take VS suggestions in changed files
dougbu added a commit to dotnet/extensions that referenced this issue Aug 22, 2019
@dougbu
Add switch to enable expected and unexpected arguments in any order (#…
e443397 
…2210)

- unblocks work on dotnet/aspnetcore#4923
  - arguments for inside and outside men of service reference doc gen tool are mixed by default
  - users may add either argument type to the end of the outside man's command line
- e.g. "command --unexpected unexpectedValue --expected" can now set the "expected" option
  - only "--unexpected" and "unexpectedValue" are added to RemainingArguments in that case

- default behaviour of the command-line parser is unchanged to avoid breaking existing applications
- new switch is supported only when calling `CommandLineApplication` constructor for top-level commands
  - `dotnet-getdocument` (the outside man) has no (sub)commands and expanding scope would increase churn

nits: take VS suggestions in changed files
dougbu added a commit that referenced this issue Aug 23, 2019
@dougbu
Fix a typo and address remaining service reference TODO items
48f7a01 
- #4923
- typo caused problems when cleaning files
- add `%(OpenApiProjectReference.GlobalPropertiesToRemove)` metadata
- address timing issues cropping up occasionally in builds using service ref features
  - avoid `AfterTargets="Build"`; referencing projects sometimes continue while post-build work is done
    - run after `CoreBuild` in inner builds and after `DispatchToInnerBuilds` in outer builds
    - do same in GetDocumentInsider.csproj
- set only properties in buildMultiTargeting\Microsoft.Extensions.ApiDescription.Server.targets
  - items not evaluated early enough to reference in all cases
- rename Microsoft.Extensions.ApiDescription.Client tasks
- remove net461 task assembly
@dougbu dougbu mentioned this issue Aug 23, 2019
Merged
dougbu added a commit that referenced this issue Aug 23, 2019
@dougbu
Fix a typo and address remaining service reference TODO items
9e0d87b 
- #4923
- typo caused problems when cleaning files
- add `%(OpenApiProjectReference.GlobalPropertiesToRemove)` metadata
- address timing issues cropping up occasionally in builds using service ref features
  - avoid `AfterTargets="Build"`; referencing projects sometimes continue while post-build work is done
    - run after `CoreBuild` in inner builds and after `DispatchToInnerBuilds` in outer builds
    - do same in GetDocumentInsider.csproj
- set only properties in buildMultiTargeting\Microsoft.Extensions.ApiDescription.Server.targets
  - items not evaluated early enough to reference in all cases
- rename Microsoft.Extensions.ApiDescription.Client tasks
- remove net461 task assembly
dougbu added a commit that referenced this issue Aug 23, 2019
@dougbu
Fix a typo and address remaining service reference TODO items (#13364)
8417429 
- #4923
- typo caused problems when cleaning files
- add `%(OpenApiProjectReference.GlobalPropertiesToRemove)` metadata
  - stop removing `$(Configuration)` or `$(Platform)` global properties
- address timing issues cropping up occasionally in builds using service ref features
  - avoid `AfterTargets="Build"`; referencing projects sometimes continue while post-build work is done
    - instead use `BeforeTargets="Build"` since that's a no-op target wrapping up a build
- set only properties in buildMultiTargeting\Microsoft.Extensions.ApiDescription.Server.targets
  - items not evaluated early enough to reference in all cases
- rename Microsoft.Extensions.ApiDescription.Client tasks
- remove net461 task assembly
@dougbu
Copy link
Contributor Author

dougbu commented Aug 23, 2019

@dougbu dougbu closed this as completed Aug 23, 2019
@dougbu dougbu added Done This issue has been fixed and removed Working labels Aug 23, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dougbu dougbu

Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed task
Projects
None yet
Milestone
3.0.0-preview9
Development

No branches or pull requests
3 participants
@dougbu @mkArtakMSFT @aspnet-hello