Skip to content

Fixed some URLs in the documentation#8078

Merged
rokonec merged 1 commit intodotnet:mainfrom
exoosh:main
Oct 26, 2022
Merged

Fixed some URLs in the documentation#8078
rokonec merged 1 commit intodotnet:mainfrom
exoosh:main

Conversation

@exoosh
Copy link
Contributor

@exoosh exoosh commented Oct 20, 2022

  • Changed http:// links to https:// where that was the case (a handful of http:// links remain), but all changed ones were checked
  • Where GitHub links changed from master to main
  • Where source lines were referenced it now points to a given revision, because pointing to a branch name means the line number may be incorrect. NB: so this was very much intentional!
  • Linked to the archived page for dead links
  • Updated old MSDN and docs.microsoft.com and blog links to point to the current location (even if it's the archive on learn.microsoft.com)
  • One wrong link (http://source.roslyn.io/#MSBuildFiles/C/ProgramFiles(x86)/MSBuild/14.0/bin_/amd64/Microsoft.Common.CurrentVersion.targets,1820) remains in ResolveAssemblyReference.md

@dnfadmin
Copy link

dnfadmin commented Oct 20, 2022 •
edited
Loading

CLA assistant check
All CLA requirements met.

@dnfadmin
Copy link

dnfadmin commented Oct 20, 2022

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ exoosh sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@exoosh
Copy link
Contributor Author

exoosh commented Oct 20, 2022

Not exactly sure why one of the two bot comments shows the CLA as signed and the other does not. Clicking "recheck" doesn't change a thing, but the link of the one saying I didn't sign it points to the exact same (signed) CLA. Odd ...

@rainersigwald
Copy link
Member

rainersigwald commented Oct 20, 2022

The check is the critical thing for the CLA, and it's passing, so we can just ignore the second comment. Sorry for the confusion!

rainersigwald
rainersigwald reviewed Oct 20, 2022
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Looks great, thank you! I made a few suggestions, the main one being: the docs/learn team prefers that URLs not have a locale (en-us), so that they can redirect to the user's best-match locale.

I think https://github.com/dotnet/msbuild/blob/v14.0.25420.1-ish/src/XMakeTasks/Microsoft.Common.CurrentVersion.targets#L1820 is a good substitute.

@exoosh
Copy link
Contributor Author

exoosh commented Oct 20, 2022

Will squash the previous commits into a signed commit and force-push to my branch. This should update the PR.

@exoosh @rainersigwald
Fixed some URLs in the documentation
a7c74b0 
- Changed http:// links to https:// where that was the case (a handful
  of http:// links remain), but all changed ones were checked
- Where GitHub links changed from `master` to `main`
- Where source lines were referenced it now points to a given revision,
  because pointing to a branch name means the line number may be
  incorrect. NB: so this was very much intentional!
- Linked to the archived page for dead links
- Updated old MSDN and docs.microsoft.com and blog links to point to the
  current location (even if it's the archive on learn.microsoft.com)
- Remove en-us from canonical links to learn.microsoft.com ...

Co-authored-by: Rainer Sigwald <raines@microsoft.com>
@exoosh
Copy link
Contributor Author

exoosh commented Oct 20, 2022

Found a handful more of the /en-us/ in the URLs and also fixed those (and tested them afterwards).

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Oct 24, 2022
@rokonec rokonec merged commit 3777dca into dotnet:main Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rainersigwald rainersigwald rainersigwald approved these changes

+1 more reviewer

@Forgind Forgind Forgind approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@exoosh @dnfadmin @rainersigwald @Forgind @rokonec