Add documentation requirements to CONTRIBUTING.md by joeyaiello · Pull Request #3794 · PowerShell/PowerShell

joeyaiello · 2017-05-16T22:50:33Z

This is intended to address the underlying issue behind #2566 by adding a note in CONTRIBUTING.md to create a corresponding pull request in PowerShell/PowerShell-Docs when making code changes that warrant it.

It's also going to take some enforcement from @PowerShell/powershell-maintainers, which I know will be difficult because our documentation isn't completely up-to-date--we're playing catch up here. However, it should be analogous to the test debt that we've had while enforcing requirements for new tests.

I'm also not opposed to creating a label as well to track those PRs where doc changes are required but where they haven't been addressed. It's really up to the @PowerShell/powershell-maintainers here, I'm open to whatever.

joeyaiello · 2017-05-16T22:51:07Z

Also, I apologize for the formatting changes. I had to make them for the file to be maintainable (bulleted lists were busted the way they were).

mirichmo · 2017-05-17T00:23:16Z

If the change ....

daxian-dbw · 2017-05-17T16:51:10Z

Better to include the PR number as well, like Update-Item now supports -FriendlyName (#1234). Otherwise, it's hard to correlate the change log with a PR.

TravisEz13 · 2017-05-17T18:11:33Z

Can we enable markdown validation before making changes?

There should be a new line before a code block and a language name.

I'm fine going through the file completely and making it lint compliant, but I thought I was supposed to hold formatting changes to another PR so as to make the review easier😉

I'll start another commit now. If you want me to drop it to another PR, I can, but it looks like everyone here is approving of the content change already.

TravisEz13 · 2017-05-17T18:12:49Z

newline after a code block

SteveL-MSFT · 2017-05-17T21:30:16Z

change may to must

SteveL-MSFT · 2017-05-17T21:31:24Z

@joeyaiello in the future, the best practice is style/formatting changes should be a separate PR otherwise it's hard to see what the actual functional change was

we should also change the PR template

TravisEz13 · 2017-05-18T00:36:33Z

#3805 for issues not introduced by this change

TravisEz13 · 2017-05-18T00:39:37Z

comma at end of line is not needed

TravisEz13 · 2017-05-18T00:40:48Z

Qualify this. For example, This requirement includes

TravisEz13 · 2017-05-18T00:42:23Z

could you remove the double the this is really bad...

TravisEz13

This document needs to go through a separate grammar scrub. I didn't see any major issues with your changes.

SteveL-MSFT · 2017-05-18T21:58:42Z

@joeyaiello please add verbage to https://raw.githubusercontent.com/PowerShell/PowerShell/master/.github/PULL_REQUEST_TEMPLATE.md for the doc requirement

iSazonov · 2017-05-23T03:04:44Z

Maybe " add parameter "?

iSazonov · 2017-05-23T06:26:00Z

It seems it is a wrong direction.

Do you really think a community contributer want to spend time writing documentation?
How many community contributers you see? You want blocks community?
Maybe you see the same process in CoreFX Repo?

The proposed process is ideal for obtaining a high-quality product in the enterprise but not in the community project.
If you want many feedbacks and contributions from community an contributer must be able to:

Install dev environment as simple as three click.
Get fast feedback on - Is the bug or new approved feature? Where is right place to fix?
Get fast PR code review.

I can not confidence say Yes for this points and now there's one more No for fourth point (docs). I may be wrong, but CoreFX Repo has a much quicker reaction.

As for me. Now @daxian-dbw @lzybkr @SteveL-MSFT spend a lot of time on me, and I hope they eventually get a profit. If I write docs, I'll spend a lot of time, and they'll waste more on fixing it. I believe the documentation should be written by professional docs writers.
The real benefit for the documentation will be from me only if I take part in reviewing the docs translation on Russion.

I believe that the process of writing documentation should be asynchronous rather than synchronous, should not be entrusted to code writers and should not block code PRs.

My vision of the process:
Mantainers:

Request high quality Issue description with clear repo steps and code samples - it can be used in docs.
Request high quality description in PRs: link to Issue if one exists, if no the Issue description should be done in the PR, a short and clear description of the fix or the enhancement - the PR description is a main base for documentation.
Mark the PR with Docs-Needed.

In this way, we will prepare a high-quality base for documenting without blocking and wasting time.

Doc writer:
4. Search PRs with Docs-Needed.
5. Open new PR in Docs Repo with link on the code PR.
6. Request review from original code PR author/code reviewers/Issue author.
7. Request to change Docs-Needed with Docs-Fixed in original code PR.

joeyaiello · 2017-05-24T18:20:20Z

@iSazonov I hear you. If others agree, we can certainly go down the documentation required label route.

TravisEz13 · 2017-05-24T18:39:47Z

I would prefer a more flexible approach that will be open to more contributors.

SteveL-MSFT · 2017-05-25T16:49:26Z

I agree with what @iSazonov is saying. Let's start with a documentation required label

joeyaiello · 2017-05-30T18:12:17Z

Okay, expect a Round 2 of this PR later this week.

SteveL-MSFT · 2017-05-31T02:13:33Z

I think we've been using Up-for-grabs mostly to indicate easy/small/non-critical issues as a way to start contributing code and not really as needing help

Sometimes it's not obvious fix, and it would be nice that maintainers accompany the comment how and where to fix, or at least where it's best to start.

You can fix this in another PR, but this doesn't have anything to do with this change.

Fair enough

SteveL-MSFT · 2017-05-31T02:16:06Z

I think we agreed to relax this and just add a Needs-Documentation label

joeyaiello · 2017-06-05T23:31:04Z

Alright, updated to reflect the label approach in Steve's PR at #3908. Would appreciate another quick review. /cc @iSazonov @SteveL-MSFT

TravisEz13 · 2017-06-06T00:41:51Z

+  a Maintainer will add the `Documentation Needed` label to your PR and add an issue to the [PowerShell-Docs repository][PowerShell-Docs],
+  so that we make sure to update official documentation to reflect your contribution.
+  As an example, this requirement includes any changes to cmdlets (including cmdlet parameters) and features which have associated about_* topics.
+  While not required, we appreciate any contributors who add this label and the issue themselves.


This sentence reads oddly. This is minor, but suggestion While not required, we appreciate any contributors who add this label and create the issue themselves.

iSazonov · 2017-06-06T04:19:55Z

+  * Instead of "New parameter 'ConnectionCredential' in New-SqlConnection",
+  write "New-SqlConnection: added parameter 'ConnectionCredential'".
+* If your change warrants an update to user-facing documentation,
+  a Maintainer will add the `Documentation Needed` label to your PR and add an issue to the [PowerShell-Docs repository][PowerShell-Docs],


In RFC Repo we seem refused Issue-based discussions and move to PR-based discussions. Here is the same - do we need duplicated Issues in Docs Repo? I still believe that doc writers can easy get an information from PowerShell Repo by "Documentation Needed" label filter and immediately create a Doc PR excluding an Doc Issue. Cross-reference links "PowerShell PR" <-> "Doc PR" will help us too.

Unfortunately, there's not a stable set of folks or workflow for getting these docs written (and actually, a lot of the preliminary writing will fall on engineers/me on the PowerShell Team), so we have to do this asynchronously with potentially extended latency.

We'll also have a lot of these things that are more than just a parameter addition (e.g. the & backgrounding) where we'll need some longer-form content. I just don't want these tasks to get lost in merged PRs.

Thanks for clarify! I'll try to help out a little.

joeyaiello · 2017-06-06T22:33:42Z

I'm okay without @HemantMahawar's review. If everything looks good, feel free to merge.

joeyaiello requested review from HemantMahawar, SteveL-MSFT, TravisEz13 and daxian-dbw May 16, 2017 22:50

msftclas added the cla-not-required label May 16, 2017

mirichmo reviewed May 17, 2017

View reviewed changes

daxian-dbw reviewed May 17, 2017

View reviewed changes

TravisEz13 reviewed May 17, 2017

View reviewed changes

joeyaiello force-pushed the DocsOnPRTemplate branch from 836bc42 to 3796bdc Compare May 17, 2017 20:58

SteveL-MSFT requested changes May 17, 2017

View reviewed changes

TravisEz13 reviewed May 18, 2017

View reviewed changes

Comment thread .github/CONTRIBUTING.md Outdated

TravisEz13 May 18, 2017

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

comma at end of line is not needed

TravisEz13 reviewed May 18, 2017

View reviewed changes

Comment thread .github/CONTRIBUTING.md Outdated

TravisEz13 May 18, 2017

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

could you remove the double the this is really bad...

TravisEz13 reviewed May 18, 2017

View reviewed changes

mirichmo assigned TravisEz13 May 18, 2017

iSazonov reviewed May 23, 2017

View reviewed changes

Comment thread .github/CONTRIBUTING.md Outdated

iSazonov May 23, 2017

Copy link
Copy Markdown

Collaborator

Choose a reason for hiding this comment

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

Maybe " add parameter "?

joeyaiello reacted with thumbs up emoji

SteveL-MSFT requested changes May 31, 2017

View reviewed changes

joeyaiello added 3 commits June 5, 2017 16:16

add doc reqs to CONTRIBUTING

c9fb943

fix CONTRIBUTING.md to pass lint

2d84cd5

last round of PR comments

cb7bb88

joeyaiello force-pushed the DocsOnPRTemplate branch from a45e526 to cb7bb88 Compare June 5, 2017 23:16

update for Documentation Needed label

36a3eba

TravisEz13 reviewed Jun 6, 2017

View reviewed changes

iSazonov reviewed Jun 6, 2017

View reviewed changes

address minor PR comments

2c9af8b

TravisEz13 approved these changes Jun 6, 2017

View reviewed changes

SteveL-MSFT approved these changes Jun 6, 2017

View reviewed changes

daxian-dbw approved these changes Jun 6, 2017

View reviewed changes

TravisEz13 merged commit a164609 into PowerShell:master Jun 7, 2017

joeyaiello deleted the DocsOnPRTemplate branch June 7, 2017 17:39

Conversation

joeyaiello commented May 16, 2017

Uh oh!

joeyaiello commented May 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxian-dbw May 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented May 17, 2017

Uh oh!

TravisEz13 commented May 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TravisEz13 May 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TravisEz13 left a comment

Choose a reason for hiding this comment

Uh oh!

SteveL-MSFT commented May 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov commented May 23, 2017

Uh oh!

joeyaiello commented May 24, 2017

Uh oh!

TravisEz13 commented May 24, 2017

Uh oh!

SteveL-MSFT commented May 25, 2017

Uh oh!

joeyaiello commented May 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joeyaiello commented Jun 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joeyaiello commented Jun 6, 2017

Uh oh!

Reviewers

Assignees

daxian-dbw May 17, 2017 •

edited

Loading

TravisEz13 May 18, 2017 •

edited

Loading