Fix XML documentation generation and add Markdown docs to VS solution #315

ghost · 2017-10-09T19:01:46Z

Bringing docs in on both sides and ignoring unstaged xml doc files

jonorossi · 2017-10-10T08:38:01Z

Castle.Core.sln

@@ -31,6 +31,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Castle.Services.Logging.Ser
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Castle Services", "Castle Services", "{A598EE9B-41CE-4BE8-BF93-2C91F919F97E}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Castle Docs", "Castle Docs", "{EC3FDC8C-1D6C-49DA-AD19-E1521A75728E}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Documentation", "docs\Documentation.csproj", "{5F7AA0D3-FE16-40BD-9492-7B8BD509FD57}"


The new tooling allows project files to support wildcards, do solution files not support wildcards for files?

It would be great if they sorted solution files out, but I have not come across anything that allows that behaviour in VS latest.

Nothing here either

jonorossi · 2017-10-10T08:39:46Z

.gitignore

+src/Castle.Core/Castle.Core.xml
+src/Castle.Services.Logging.NLogIntegration/Castle.Services.Logging.NLogIntegration.xml
+src/Castle.Services.Logging.SerilogIntegration/Castle.Services.Logging.SerilogIntegration.xml
+src/Castle.Services.Logging.log4netIntegration/Castle.Services.Logging.log4netIntegration.xml


Oh, don't these usually get written to the output bin directory next to the DLL and PDB files? That is what VS used to default to before the new CLI tooling.

I was almost sure that OutputPath was the way to do it. But it isn't working here for some reason.

I'm not sure what is different to Windsor because Windsor 4.1.0 has XML files for each target framework in their bin/debug or bin/release and in the NuGet packages, and it doesn't specify the DocumentationFile element.

We need to fix this because I just realised that if the file is getting written here than wouldn't that mean that the last target to get built becomes the output for all targets, i.e. if netstandard1.6 is built last than that's the docs for all targets.

Will have a fix for this soon.

jonorossi · 2017-10-16T13:43:04Z

src/Castle.Core/Castle.Core.csproj

@@ -19,6 +19,7 @@
 		<PublicSign Condition="'$(OS)'=='Unix'">true</PublicSign>
 		<FrameworkPathOverride Condition="'$(OS)'=='Unix'">$(NuGetPackageFolders)microsoft.targetingpack.netframework.v4.6.1\1.0.1\lib\net461\</FrameworkPathOverride>
 		<PackageTags>castle dynamicproxy dynamic proxy dynamicproxy2 dictionaryadapter emailsender</PackageTags>
+		<OutputPath>bin\$(Configuration)\$(TargetFramework)\</OutputPath>
 		<DocumentationFile>$(OutputPath)Castle.Core.xml</DocumentationFile>


Will have a fix for this soon.

I thought the fix was going to be removing the DocumentationFile since Windsor has XML docs in its packages without this element.

Yes, it is actually

<GenerateDocumentationFile>true</GenerateDocumentationFile>

I must have missed that earlier.

…instead from common.props file

jonorossi

Thanks for fixing it in common.props, forget to check there just assumed those two files matched between both projects.

jonorossi · 2017-10-18T15:36:17Z

.gitignore

+
+# Docs artifacts
+docs/obj/
+docs/bin/


Maybe we should just ignore obj and bin directories everywhere, we've got 4 sets of these patterns now.

jonorossi · 2017-10-18T15:36:53Z

.gitignore

+docs/bin/
+
+# Xml doc/test files
+*.xml


I don't think ignoring all XML files everywhere is a good idea, this isn't needed anymore anyway.

Mind you, this is only the root folder. It also prevents DesktopClrTestResults.xml and NetCoreClrTestResults.xml from being unstaged and accidentally committed.

jonorossi · 2017-11-23T15:59:24Z

@Fir3pho3nixx ping

ghost · 2017-11-24T19:18:59Z

@jonorossi pong

jonorossi · 2017-11-30T13:02:37Z

.gitignore

 tools/**/build/
+
+# Xml doc/test files
+*.xml


Mind you, this is only the root folder. It also prevents DesktopClrTestResults.xml and NetCoreClrTestResults.xml from being unstaged and accidentally committed.

@Fir3pho3nixx I think you misunderstand how gitignores work, sorry for this pull request being death by a thousand cuts.

If the pattern does not contain a slash /, Git treats it as a shell glob pattern and checks for a match against the pathname relative to the location of the .gitignore file (relative to the toplevel of the work tree if not from a .gitignore file).

If the pattern ends with a slash, it is removed for the purpose of the following description, but it would only find a match with a directory. In other words, foo/ will match a directory foo and paths underneath it, but will not match a regular file or a symbolic link foo (this is consistent with the way how pathspec works in general in Git).

A leading slash matches the beginning of the pathname. For example, "/*.c" matches "cat-file.c" but not "mozilla-sha1/sha1.c".

(https://git-scm.com/docs/gitignore)

*.xml is a glob pattern across all directories (like the *.suo pattern), it would need to be /*.xml to just match the root.

There is also no need for the **, you can just do bin/ to match the directory rather the contents.

Okiday, I checked the globs by running git status and it appeared to be OK. Will fix.

Cheers, I know I'm a nitpicking bastard 😉

P.S. this PR now also conflicts because of another PR I merged earlier.

Don't worry I will sort that.

@jonorossi - I think I misled you earlier when I said the XML artifacts are only in the root.

I cleaned the project, using git clean -x -f -d, then changed the .gitignore to have a leading slash in front of the *.xml -> /*.xml.

After running a full build and tests, I then double checked using git status with the following result.

Your branch is up-to-date with 'origin/harmonising-core-and-windsor-2'. All conflicts fixed but you are still merging. (use "git commit" to conclude merge) Changes to be committed: modified: CHANGELOG.md modified: src/Castle.Services.Logging.log4netIntegration/Castle.Services.Logging.log4netIntegration.csproj Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory) modified: .gitignore modified: Castle.Core.sln.DotSettings Untracked files: (use "git add <file>..." to include in what will be committed) src/Castle.Services.Logging.log4netIntegration/Castle.Services.Logging.Log4netIntegration.xml

Notice how git thinks it needs to add the xml build artifact from the Log4netIntegration?

I believe my original approach is correct because this is a build artifact. It was my fault for not being clear on this in the first place but I do test my work before committing(admittedly I don't always test it on linux hence the previous casing issue). I am also puzzled as to why the casing issue cropped up and was not detected by Travis. Bizarre.

The globbing pattern I thought would be OK because of this:

Two consecutive asterisks ("**") in patterns matched against full pathname may have special meaning: A leading "**" followed by a slash means match in all directories. For example, "**/foo" matches file or directory "foo" anywhere, the same as pattern "foo". "**/foo/bar" matches file or directory "bar" anywhere that is directly under directory "foo". <- !! Why I put the glob on the front of bin/obj. !! A trailing "/**" matches everything inside. For example, "abc/**" matches all files inside directory "abc", relative to the location of the .gitignore file, with infinite depth. <- !! Why I put the glob on the end !! A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, "a/**/b" matches "a/b", "a/x/b", "a/x/y/b" and so on. Other consecutive asterisks are considered invalid.

You might consider it unnecessary although it is still valid. I just failed to explain myself perhaps properly.

If none of this is good for you, my alternative is to close out the PR. I am done wasting time on this.

ghost · 2018-01-17T22:27:47Z

I will harmonise the projects after we get this through: castleproject/Windsor#351

jonorossi · 2018-01-22T14:58:31Z

I've committed 5a9bcab that fixes XML documentation file generation to use GenerateDocumentationFile to avoid the race condition between building multiple TFMs, and also fixes shipping the same XML file for each TFM because each one overwrites the previous.

Sorry for not realising you were talking about 3 different XML files from the unit test execution, we obviously want the XML doc files written to the bin directories paired with their DLL and PDB. I've explicitly ignored those 3 other XML files so if we add an XML file for something else it'll show up to be committed.

jonorossi requested changes Oct 10, 2017

View reviewed changes

jonorossi added this to the vNext milestone Oct 13, 2017

Bringing docs into the solution and fixing xml document paths

0609205

jonorossi reviewed Oct 16, 2017

View reviewed changes

Gavin van der Merwe added 3 commits October 17, 2017 20:00

Fixing xml docs by using global property 'GenerateDocumentationFile' …

39b4559

…instead from common.props file

Merge branch 'xml-docs-fix' into harmonising-core-and-windsor-2

57a10b4

Reformat of common.props

c71dbff

jonorossi requested changes Oct 18, 2017

View reviewed changes

jonorossi changed the title ~~Harmonise Castle Core and Windsor~~ Fix XML documentation generation and add Markdown docs to VS solution Oct 18, 2017

Gavin van der Merwe added 2 commits November 24, 2017 19:02

Cleaned up gitignore file

f40c170

Bumping dotnet core to try and make travis ci build pass

e4485ac

jonorossi reviewed Nov 30, 2017

View reviewed changes

Fixed conflict so we can merge docs

8ef9eec

ghost closed this Jan 17, 2018

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix XML documentation generation and add Markdown docs to VS solution #315

Fix XML documentation generation and add Markdown docs to VS solution #315

ghost commented Oct 9, 2017

jonorossi Oct 10, 2017

ghost Oct 10, 2017

jonorossi Oct 10, 2017

ghost Oct 10, 2017

jonorossi Oct 13, 2017

ghost Oct 14, 2017

jonorossi Oct 16, 2017

ghost Oct 17, 2017

jonorossi left a comment

jonorossi Oct 18, 2017

jonorossi Oct 18, 2017

ghost Nov 24, 2017

jonorossi commented Nov 23, 2017

ghost commented Nov 24, 2017

jonorossi Nov 30, 2017

ghost Dec 7, 2017

jonorossi Dec 7, 2017

ghost Dec 7, 2017

ghost Jan 6, 2018 •

edited by ghost

Loading

ghost commented Jan 17, 2018

jonorossi commented Jan 22, 2018

Fix XML documentation generation and add Markdown docs to VS solution #315

Fix XML documentation generation and add Markdown docs to VS solution #315

Conversation

ghost commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonorossi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonorossi commented Nov 23, 2017

ghost commented Nov 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jan 6, 2018 • edited by ghost Loading

Choose a reason for hiding this comment

ghost commented Jan 17, 2018

jonorossi commented Jan 22, 2018

ghost Jan 6, 2018 •

edited by ghost

Loading