sp-dev-docs icon indicating copy to clipboard operation
sp-dev-docs copied to clipboard

Improvements

Open yihezkel opened this issue 3 years ago • 2 comments

I only made the fixes that I cold do quickest; there's much to do to improve this article.

Category

  • [ ] Content fix
  • [ ] New article
  • [x] Example checked item (delete this line)

DELETE THIS LINE BEFORE SUBMITTING | For the above list, an empty checkbox is [ ] as in [SPACE]. A checked checkbox is [x] with no space between the brackets. Use the PREVIEW tab at the top right to preview the rendering before submitting your issue.

Related issues

  • fixes #issuenumber
  • partially #issuenumber
  • mentioned in #issuenumber

DELETE THIS LINE BEFORE SUBMITTING | If this fixes (aka: closes) or references an issue, please reference it here. This helps maintaining the issue list as it will (1) link the PR to the issue & (2) automatically close the issue when this PR is merged in.

What's in this Pull Request?

DELETE THIS LINE BEFORE SUBMITTING | Please describe the changes in this PR. Sample description or details around bugs which are being fixed.

Submission guidelines

  • !!IMPORTANT!! - All submissions must complete the baseline sections included in this template. Ignoring or deleting this template may result in closing the issue with the label type:incomplete-submission.
  • Follow our guidance on How To Create Good Pull Requests.
  • Target the main branch of this repo.
  • When changing a page, ensure you update the ms.date front matter wih the current date in the format MM/DD/YYYY.
  • Review all build checks and address the automated errors, warnigns, and suggestions.
  • NOTE: The live site is based on the live branch. Site owners periodically refresh live branch from the main branch so merged PRs won't immediately appear on the live site. Please be patient to see your changes appear on the live site.

DELETE THIS SECTION BEFORE SUBMITTING

yihezkel avatar Sep 11 '22 09:09 yihezkel

Docs Build status updates of commit 1d6f171:

:white_check_mark: Validation status: passed

File Status Preview URL Details
docs/solution-guidance/security-apponly-azuread.md :white_check_mark:Succeeded View

For more details, please refer to the build report.

Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.

For any questions, please:

VesaJuvonen avatar Sep 11 '22 09:09 VesaJuvonen

Also, in the future, please follow the instructions in the PR template when submitting a new PR. In the future, ignoring the instructions/guidance in the PR template will result in rejecting the PR without review. It helps us review submissions in a timely manner.

andrewconnell avatar Sep 23 '22 14:09 andrewconnell

Closed to no activity/response by author

andrewconnell avatar Oct 20 '22 11:10 andrewconnell

@andrewconnell "Also, in the future, please follow the instructions in the PR template when submitting a new PR. In the future, ignoring the instructions/guidance in the PR template will result in rejecting the PR without review. It helps us review submissions in a timely manner."

I think this is a bit harsh. I didn't ignore the instructions. I tried to follow the steps, but apparently I missed one (the date). Is it really better to reject a PR with good suggestions to improve our docs because of a step missing?

yihezkel avatar Oct 20 '22 15:10 yihezkel

@andrewconnell "Closed to no activity/response by author" I apologize, just got back this week after 3 weeks off. Do you not want any of these docs improvements?

yihezkel avatar Oct 20 '22 15:10 yihezkel

@yihezkel said:

I think this is a bit harsh. I didn't ignore the instructions. I tried to follow the steps, but apparently I missed one (the date). Is it really better to reject a PR with good suggestions to improve our docs because of a step missing?

Sorry if it came across as harsh... wasn't the intent. The PR template has sections to fill in the blanks as you can see above... things to indicate what the PR is about, what changes it includes if it's a content update or a new doc, etc. None of that stuff was addressed and seemed ignored, meaning it takes longer for reviewers to understand what this is about.

That comment was simply included as a request as this seemed to be your first PR submission.

But with no response for nearly 4 weeks after a review & a request for changes, the issue was closed as it seemed we'd not get any follow-up to the requested changes.

Do you not want any of these docs improvements?

We do, but as you said, there are a lot of changes to apply to the doc as it's outdated. We'd rather have a complete update than a partial one because a partial one implies it's been updated and is now current. That confuses consumers of the doc who think "oh, if this was updated in October, then everything is current".

andrewconnell avatar Oct 20 '22 17:10 andrewconnell