Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Find IDocumentProvider using a more-laborious process #8590

Merged
merged 1 commit into from
Oct 27, 2018

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Oct 11, 2018

  • Type.GetType(string) requires an assembly-qualified name and we don't know the assembly

nit:

  • reflect recent change to dotnet-getdocument's Resources.resx file in its designer file

@dougbu
Copy link
Member Author

dougbu commented Oct 11, 2018

Would like to get this into the feature branch soon 😃

@dougbu dougbu changed the base branch from feature/client.code.generation to release/2.2 October 11, 2018 23:37
@dougbu
Copy link
Member Author

dougbu commented Oct 11, 2018

Changed base now that I've merged feature/client.code.generation into release/2.2

@dougbu dougbu force-pushed the dougbu/not.type.gettype branch from d2d253d to 6559a1b Compare October 11, 2018 23:42
@dougbu
Copy link
Member Author

dougbu commented Oct 11, 2018

Added fix for #8593

@rynowak or @pranavkm this is what we've been discussing offline. Please have a look.

@dougbu dougbu force-pushed the dougbu/not.type.gettype branch from 6559a1b to 064ec5c Compare October 14, 2018 01:41
@dougbu
Copy link
Member Author

dougbu commented Oct 14, 2018

🆙📅
With one of the fixes discussed offline w.r.t. Startup.Configure(...), everything is again working end-to-end. Not including anything more here until that discussion completes.

Copy link
Member

@rynowak rynowak left a comment

Choose a reason for hiding this comment

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

Seems pretty good, I have a few small comments

Copy link
Member Author

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Couple of questions for you two, @pranavkm and @rynowak

@dougbu dougbu force-pushed the dougbu/not.type.gettype branch from 064ec5c to dcbd780 Compare October 25, 2018 19:21
@dougbu
Copy link
Member Author

dougbu commented Oct 25, 2018

🆙📅 @rynowak do you want to take one more look?

- #8593
- also find `IDocumentProvider` using a more-laborious process
  - `Type.GetType(string)` requires an assembly-qualified name and we don't know the assembly
- default method name now `GenerateAsync`
- only supported signature is `public Task GenerateAsync(string, TextWriter)`

also:
- handle more error cases in the tool's inside man
- avoid an empty document file if `IDocumentProvider.GenerateAsync(...)` fails
- unwrap an `AggregateException`

nits:
- remove duplicate comments
- change `GetDocumentCommandWorker.TryProcess(...)` to return `false` on failure
  - minor because return value is currently ignored
- rename `GetDocumentCommandContext.Output` -> `OutputPath`
- reflect recent change to `dotnet-getdocument`'s `Resources.resx` file in its designer file
@dougbu dougbu force-pushed the dougbu/not.type.gettype branch from dcbd780 to 7ae3578 Compare October 27, 2018 19:33
@dougbu dougbu merged commit 37e5629 into release/2.2 Oct 27, 2018
@dougbu dougbu deleted the dougbu/not.type.gettype branch October 27, 2018 22:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants