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

Update documentation link to point to docs.nunit.org #81

Merged
merged 3 commits into from
Jul 13, 2020

Conversation

SeanKilleen
Copy link
Member

@SeanKilleen SeanKilleen commented Jul 7, 2020

Submitted for your consideration, @rprouse.

This PR removes the documentation page and instead points the documentation to https://docs.nunit.org.

@SeanKilleen SeanKilleen changed the title WIP: Update documentation link to point to docs.nunit.org Update documentation link to point to docs.nunit.org Jul 7, 2020
@SeanKilleen
Copy link
Member Author

@rprouse pinging on this only in case you missed it in the recent flurry of updates. No rush.

@CharliePoole
Copy link
Contributor

The web site is kind of messed up right now @rprouse. Since you're the only one who can update it, could you review this soon? I think you should decide whether to eliminate the page as proposed or just update the links it contains.

@@ -17,12 +17,12 @@
<ul class="nav navbar-nav">
<li><a href="/news/"><i class="fa fa-commenting-o"></i> News</a></li>
<li><a href="/download/"><i class="fa fa-download"></i> Download</a></li>
<li><a href="/documentation/"><i class="fa fa-book"></i> Documentation</a></li>
<li><a href="https://docs.nunit.org" target="_blank"><i class="fa fa-book"></i> Documentation</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

I think we can merge this without a needing Rob’s explicit approval. 😊 Do we definitely want the target=blank? I would have thought docs should be a more natural part of the site, rather than something like Twitter - which is a new tab thing.

No strong opinions however. 😊

Copy link
Member Author

Choose a reason for hiding this comment

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

@ChrisMaddock I don't have a strong opinion either way. I tend to use a blank target when I know the look & feel of the site will be different, so that it's clear it's a different entity entirely. I've got no issues if we'd prefer to remove that. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd go by the availability of the top level menu. If it's not there then open in a new tab.

Copy link
Member

Choose a reason for hiding this comment

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

That seems a sensible convention. 🙂

@ChrisMaddock
Copy link
Member

Oops, @CharliePoole - we cross-posted! Is there something specific that you think needs Robs decision here, sorry?

@CharliePoole
Copy link
Contributor

Well, to my knowledge, our working agreement has Rob as the sole maintainer. Of course, many of us have GitHub permission, but that's more as a backup in case Rob is hit by s bus. That said, if you have had different signals from Rob, feel free. I'm just not comfortable doing it myself.

@ChrisMaddock
Copy link
Member

Thanks Charlie. I've dropped you an email to further explain some of my thoughts here.

In my personal opinion, I'm happy that Rob's been sufficiently involved in the discussions around this docs system to express his intentions - so I'll go ahead and merge this to unblock Sean. Rob - apologies if I've mis-judged the situation!

@ChrisMaddock ChrisMaddock merged commit 8eaf802 into nunit:master Jul 13, 2020
@rprouse
Copy link
Member

rprouse commented Jul 15, 2020

Thanks for taking care of this @ChrisMaddock

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.

4 participants