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

Add GitHub PRs to the Static Analysis integrations #20981

Merged
merged 37 commits into from
Jan 5, 2024

Conversation

jhgilbert
Copy link
Contributor

What does this PR do? What is the motivation?

DOCS-5928, adds GitHub PRs to the integrations for Static Analysis.

Merge instructions

  • Please merge after reviewing

Additional notes

@jhgilbert jhgilbert added WORK IN PROGRESS No review needed, it's a wip ;) Do Not Merge Just do not merge this PR :) labels Dec 12, 2023
@jhgilbert jhgilbert requested a review from a team as a code owner December 12, 2023 00:15
Copy link
Contributor

github-actions bot commented Dec 12, 2023

@jhgilbert jhgilbert removed WORK IN PROGRESS No review needed, it's a wip ;) Do Not Merge Just do not merge this PR :) labels Dec 12, 2023
@jhgilbert
Copy link
Contributor Author

Should not be merged yet, but is ready for docs team review.

@jhgilbert jhgilbert added WORK IN PROGRESS No review needed, it's a wip ;) Do Not Merge Just do not merge this PR :) labels Dec 20, 2023
@jhgilbert jhgilbert requested a review from a team as a code owner December 21, 2023 00:15
@github-actions github-actions bot added the Architecture Everything related to the Doc backend label Dec 21, 2023
@github-actions github-actions bot added the Images Images are added/removed with this PR label Dec 21, 2023
@jhgilbert jhgilbert added WORK IN PROGRESS No review needed, it's a wip ;) and removed WORK IN PROGRESS No review needed, it's a wip ;) Do Not Merge Just do not merge this PR :) labels Jan 2, 2024
@jhgilbert jhgilbert removed the WORK IN PROGRESS No review needed, it's a wip ;) label Jan 2, 2024
@dastrong
Copy link
Contributor

dastrong commented Jan 3, 2024

Had some feedback on this @markazerdd @jhgilbert

From the user's POV, the content in the first and third sections below have a lot of overlap. We could put the prerequisite blurb under the "Configure a GitHub App" heading?
Screenshot 2024-01-02 at 6 57 13 PM

For Step 4 of the "Subscribe to events", we check the "Pull Request review comment" box which is wrong. We need users to check the "Pull Request" box only.
Screenshot 2024-01-02 at 7 03 21 PM

Copy link
Contributor

@dastrong dastrong left a comment

Choose a reason for hiding this comment

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

Added my review comment separately, but there's one change required.

@jhgilbert
Copy link
Contributor Author

@dastrong Thanks for the feedback -- I think the user will be frustrated if they work through the prerequisite link and then have to revisit the same GitHub App to update it, so I rephrased that part to keep the user on the page instead. Let me know if this works better for you.

CC @markazerdd

Copy link
Contributor

@dastrong dastrong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@alai97 alai97 left a comment

Choose a reason for hiding this comment

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

Looks good, some copy suggestions + small anchor link fix!

content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
content/en/static_analysis/github_pull_requests.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alai97 alai97 left a comment

Choose a reason for hiding this comment

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

Looks good!

@jhgilbert jhgilbert merged commit a04606c into master Jan 5, 2024
11 of 12 checks passed
@jhgilbert jhgilbert deleted the jen.gilbert/DOCS-5928-github-static-analysis branch January 5, 2024 20:43
MaelNamNam pushed a commit that referenced this pull request Jan 17, 2024
* Add GitHub PRs to the integrations

* Update _index.md

* Update content/en/continuous_integration/static_analysis/_index.md

Co-authored-by: Mark Azer <[email protected]>

* Update _index.md

* Write feature overview for GitHub source code management

* Sketch in content outline

* Add page to the TOC

* Add section on configuring a GitHub App

* Add product shot

* Update product screenshot

* Flesh out GitHub App instructions

* Finish first draft of instructions

* Update content/en/static_analysis/github_source_code_management.md

Co-authored-by: Mark Azer <[email protected]>

* Update content/en/static_analysis/github_source_code_management.md

Co-authored-by: Mark Azer <[email protected]>

* Update content/en/static_analysis/github_source_code_management.md

Co-authored-by: Mark Azer <[email protected]>

* Update content/en/static_analysis/github_source_code_management.md

Co-authored-by: Mark Azer <[email protected]>

* Update content/en/static_analysis/github_source_code_management.md

Co-authored-by: Mark Azer <[email protected]>

* Integrate feedback

* Rename file

* Update URL

* Integrate feedback

* Update identifier

* Update _index.md

* Tweak instructions

* Integrate feedback

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Update content/en/static_analysis/github_pull_requests.md

Co-authored-by: Austin Lai <[email protected]>

* Spacing Nit

---------

Co-authored-by: Mark Azer <[email protected]>
Co-authored-by: Austin Lai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Everything related to the Doc backend editorial review Waiting on a more in-depth review Images Images are added/removed with this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants