Skip to content

Conversation

@BillWagner
Copy link
Member

This addresses the final feedback in PR #1073. I’ll close that PR as this replaces it.

Important: Please to a merge commit instead of a squash commit when we merge this. We want to preserve the original PR commits, including the committer

Update console-teleprompter.md

Update console-teleprompter.md

Update console-teleprompter.md
@saldana
Copy link

saldana commented Nov 6, 2016

Open Publishing Build Service: The pull request content has been published and here are some changed files links of this time:

Status: Succeeded.
Open Publishing Report.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • I think Program should be formatted as code
  • "It's" should be lowercase, since it is part of the sentence now

@BillWagner
Copy link
Member Author

@svick I made the suggested updates.
I also squashed to make a clean history to merge.

@saldana
Copy link

saldana commented Nov 7, 2016

✅ Validation status: passed

File Status Preview URL Details
docs/csharp/tutorials/console-teleprompter.md ✅Succeeded View Details

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better to change File to @System.IO.File so that it's resolved as a link.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here again, it might be better to change these from IDisposable to @System.IDisposable, and from Dispose to @System.IDisposable.Dispose so that they resolve as links.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here again, FIle.OpenText could be File.OpenText so it resolves as a link. Similarly, @System.IO.StreamReader instead of StreamReader.

Copy link
Contributor

Choose a reason for hiding this comment

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

Task-returning. Task could be a link -- @System.Threading.Tasks.Task.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe resolve this as a link -- @System.Threading.Tasks.Task.Wait.
@BillWagner

Copy link
Contributor

Choose a reason for hiding this comment

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

Here the link would be helpful -- @System.Action. Also, perhaps "Action delegate"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with a link to @System.Console.ReadKey?

Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with a link to Task.WhenAny?

@BillWagner
Copy link
Member Author

Responded to @rpetrusha feedback and re-squashed.

@BillWagner
Copy link
Member Author

@rpetrusha I made all your suggested changes. Do you want to review again, or is it ready to ship?

@saldana
Copy link

saldana commented Nov 8, 2016

✅ Validation status: passed

File Status Preview URL Details
docs/csharp/tutorials/console-teleprompter.md ✅Succeeded View Details

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@rpetrusha
Copy link
Contributor

@BillWagner I'll review it again quickly.

Copy link
Contributor

Choose a reason for hiding this comment

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

typo: @System.IDisposable

Copy link
Contributor

Choose a reason for hiding this comment

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

should be @System.IO.File.OpenText

Copy link
Contributor

Choose a reason for hiding this comment

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

typo: @System.IO.StreamReader

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't resolve. Should be @System.Threading.Tasks.Task.WhenAny(System.Threading.Tasks.Task[]).

@rpetrusha
Copy link
Contributor

@BillWagner A few links didn't resolve. For the last one, you could also use Task.WhenAny for simplicity, though a link to method would be nice. Once those issues are addressed, LGTM.

Also, updated the freshness date, and add the ms.author metadata tag.

Respond to updated feedback

Respond to @rpetrusha feedback.

Fix a few broken links
@BillWagner BillWagner closed this Nov 8, 2016
@BillWagner BillWagner reopened this Nov 8, 2016
@saldana
Copy link

saldana commented Nov 8, 2016

✅ Validation status: passed

File Status Preview URL Details
docs/csharp/tutorials/console-teleprompter.md ✅Succeeded View Details

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@BillWagner
Copy link
Member Author

rebasing and merging.

Thanks @rpetrusha

@BillWagner BillWagner merged commit 325c8a9 into dotnet:master Nov 8, 2016
@BillWagner BillWagner deleted the formatting-console-teleprompter-1073 branch November 8, 2016 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants