Skip to content

Conversation

nschonni
Copy link
Contributor

No description provided.

@nschonni nschonni changed the title chore: Replace old PowerBI urls chore: Cleanup redirect.json Jul 10, 2019
@nschonni
Copy link
Contributor Author

@MightyPen this is everything besides the redirects from #2452 so that part can be reviewed separately after.

@MightyPen
Copy link
Contributor

@nschonni Okay Nick, I am comfortable approving this narrowed public PR 2474. GitHub is able to display this Diff to me. And the changes all look good.
I notice the changes include fixing at least three invalid redirects so that they now will work. Very nice, thanks.

In the future, for large PRs, it is best if we do not co-mingle in the same PR nonessential removals of extra spaces with other more substantial changes. GitHub's Diff UI has recently enabled us to tell the Diff display to ignore "whitespace" changes, but that is a misnomer because the deletion of a CR-LF remains in the display. The Diff of this PR 2474 was enlarged a lot by the inclusion of nonessential blank line removals. For PRs or substantial size, it is best to remove the spaces and blank lines in their own narrowly scoped PR. Thanks.


Per our earlier discussion, I still need to test the index.md .op redirection plan, with a small scale change in live Docs. And then I need to report my Test results back to to you. I expect to report back to you next week.
If my test results are happy, then it would be great if you could proceed with a second PR that narrowly focuses on just the index.md replacements.


I am now Merging this public PR 2474. :-)

@MightyPen MightyPen merged commit 6a9a89a into MicrosoftDocs:live Jul 10, 2019
@nschonni nschonni deleted the redirect branch July 10, 2019 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants