-
Notifications
You must be signed in to change notification settings - Fork 6k
10). Add topic files 15 to 18 #17100
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.
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
10). Add topic files 15 to 18 #17100
Conversation
c4434f8
to
de825b7
Compare
Thanks @jameshkramer I gave this a good review today. A few comments: First, there are a number of warnings for the additions in the new TOC in the standard folder. Otherwise, it looks great. |
I'll try to get this reviewed later today. I think we can have a separate PR that removes the entire nodes from the previous locations. We usually have some warnings happening until all articles are merged but I believe this one has some extra ones that need to be dealt with before merging. |
@mairaw this PR is not intended for the master and should be retargered? |
@pkulikov, yes, sorry, the target should be linq-to-xml, not master, and I have now corrected it. I wonder when I will stop making this mistake. cc: @mairaw, @BillWagner |
Bill, thanks for reviewing these. I'm glad you find them suitable. Maira, I appreciate you're taking a look also. I'll look at the redirection warnings. I plan three more PRs this week, with 12 more articles. |
Apparently the great majority of the warnings are for "file not found", because the TOC refers to files that haven't been merged yet. These will be fixed over time. However the first four are "redirection url found conflict", and the message says "appears twice or more in the redirection mappings". It seems these result from there being redirections to the merged file in /standard from both the C# and the VB versions. Do we need to do something here, or just ignore the messages? |
Some of the warnings will still have to be corrected like |
But what is the fix? I thought we wanted both the C# and the VB versions to be redirected to the merged version. |
de825b7
to
3465c20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jameshkramer!
@mairaw
This PR is for user story #1576037.
It adds four topic files, makes corresponding deletions from the C# and VB folders, and adds redirection entries. There are also minor fixes to a few other files.
cc: @tfosmark
Contributes to #4728