Skip to content
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

Updating the documentation for NUnit 4, a start..... #799

Merged
merged 16 commits into from
Oct 9, 2023

Conversation

OsirisTerje
Copy link
Member

@OsirisTerje OsirisTerje commented Oct 4, 2023

I have started on the doc changes needed for NUnit 4.
It is quite a mouthful, and I have trouble swallowing.
Also, @SeanKilleen , I might need quite a bit of help getting the links, the folders, and more into a manageable state. I have made a ton of changes, and I feel I lost control in a few places.

We should also get rid of all the docs referring to Changes as being from NUnit 2 to NUnit 3. Either delete them or move them into a separate deprecated or outdated section.

@OsirisTerje
Copy link
Member Author

@SeanKilleen Step 2 fails: Too many link fails it seems... help please!

@SeanKilleen
Copy link
Member

SeanKilleen commented Oct 4, 2023

@OsirisTerje I wasn't expecting this in the queue today, sorry. I'm tied up with work stuff through this evening and likely won't be able to look at it until tomorrow morning Eastern at the earliest.

@SeanKilleen
Copy link
Member

@OsirisTerje the real issues are showing up in the first ~35 lines of build output on that step. They show the missing links and missing TOC files. The rest of the build output happens because we have a bunch of legacy docs files without templates, and I haven't figured out how to suppress it yet.

FWIW, going forward you may want to open the docs in a GitHub codespace or in a dev container in VS code. That way you can make smaller changes and run "build" within that environment in order to pinpoint these more easily.

@OsirisTerje
Copy link
Member Author

OsirisTerje commented Oct 5, 2023

@SeanKilleen I fixed the errors, and step 2 says it is a success, but with one warning, and is still marked red ??
And I cant see any warning there, except for the tons of ignored files for nunit 2.

@SeanKilleen
Copy link
Member

@OsirisTerje I'll look into it.

I also want to ensure we address more holistic issues before merging too. One example: if pages are moving, we need to ensure we are providing appropriate redirects within docfx so users aren't seeing broken links.

Marking this as "request changes" just to make sure it doesn't accidentally get merged. I think I'll be able to look at it in an hour or so.

Copy link
Member

@SeanKilleen SeanKilleen left a comment

Choose a reason for hiding this comment

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

Marking as request changes to make sure this doesn't accidentally get merged before I review it

@SeanKilleen
Copy link
Member

@OsirisTerje I'm finally starting to look at this. Thank you for your patience!

This has the happy coincidence of removing a great deal of the build output that was annoying @OsirisTerje and me :)
Based on warnings I was seeing in the container startup and then following directions at NodeSource
@SeanKilleen
Copy link
Member

Added some thoughts for review. Adding redirects for the moved files next, at which point this will be ready to merge if we choose to do so.

@SeanKilleen
Copy link
Member

@OsirisTerje I thought we had to do a lot more redirects but I saw that the titles of the URLs were changed for classic asserts, vs the URL location themselves. 👍 So I think after the remaining comments are worked through, this will be good to go

@SeanKilleen SeanKilleen self-requested a review October 9, 2023 01:34
Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Some small issues.

I also added a commit to remove compile time warnings from the example code.

OsirisTerje and others added 5 commits October 9, 2023 15:43
Co-authored-by: Sean Killeen <SeanKilleen@gmail.com>
Co-authored-by: Sean Killeen <SeanKilleen@gmail.com>
@OsirisTerje OsirisTerje merged commit abcd03f into master Oct 9, 2023
7 checks passed
@OsirisTerje OsirisTerje deleted the nunit4changes branch October 9, 2023 15:05
github-actions bot pushed a commit that referenced this pull request Oct 9, 2023
* Updating the documentation for NUnit 4, a start.....

* fixes

* update

* update

* update

* Update docfx action (and thus docfx) to latest

This has the happy coincidence of removing a great deal of the build output that was annoying @OsirisTerje and me :)

* Update the Node installer

Based on warnings I was seeing in the container startup and then following directions at NodeSource

* fix cross-reference

* fix indending on nunit 4 doc

* Add redirects for moved files

* Remove warnings from Example code

* Update docs/articles/nunit/writing-tests/Assumptions.md

Co-authored-by: Sean Killeen <SeanKilleen@gmail.com>

* Update docs/articles/nunit/Towards-NUnit4.md

Co-authored-by: Sean Killeen <SeanKilleen@gmail.com>

* update

* update

* update

---------

Co-authored-by: Sean Killeen <SeanKilleen@gmail.com>
Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com> abcd03f
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.

None yet

3 participants