Adding PR Guidelines to workflow

[jgonz120]

Bug

Fixes:

Description

Updating the Worklfow.md so the PR section is structured as different topics and guidelines.

PR Checklist

• Meaningful title, helpful description and a linked NuGet/Home issue
• Added tests
• Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

[Copilot]

PR Overview

This PR updates the workflow documentation to provide clearer, more organized guidelines for pull requests and code reviews.

• Updated the header from "Workflow" to "Workflow Guidelines".
• Reorganized and added sections to better delineate guidelines for requesting, addressing, and merging pull requests.
• Improved the clarity of feedback instructions with concrete examples.

Reviewed Changes

File	Description
docs/workflow.md	Restructured sections and updated guidelines to improve clarity.

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[donnie-msft]

I see this as being actionable. Requiring a reviewer to do the investigation and provide exact steps to remediate seems unreasonable. Perhaps a better example can be added?

[jgonz120]

I would say that it isn't actionable feedback. While the author might now know the button isn't working, the reviewer hasn't shared the context as to why it wouldn't work. For this example, I don't think there's really any additional investigation if a button is missing a listener.

[jebriede]

In more general terms, comments such as "this code won't work" are not very actionable code review comments.

According to Copilot:

The comment "this button won't do anything" is not very actionable because it doesn't provide specific details about why the button won't work or what needs to be done to fix it. An actionable code review comment should offer clear, constructive feedback that helps the developer understand the issue and how to resolve it.

An alternative, more actionable comment could be:

"This button won't do anything because the click event handler is not defined. You need to add an event listener for the button's click event that triggers the desired action."

This comment is actionable because it:

Identifies the specific issue (missing click event handler).
Provides a suggestion for how to fix the issue (adding an event listener).
Would you like more examples of actionable code review comments or any other assistance with code reviews?

[donnie-msft]

If someone submits a PR with a button not working, I don't expect we should have to go figure out what they didn't do to make the button work.

While the author might now know the button isn't working

This tells me the PR author did not do basic UI testing on their work before opening the PR. Therefore, adding this as PR guidance is not an acceptable standard to me.

Since you're also tracking metrics on review time, note that such practice will also be increasing time-to-complete PRs.

[jgonz120]

I pulled up some examples from ChatGPT:

Unactionable Feedback: ❌ "Your code is messy."
Actionable Feedback: ✔️ "Consider organizing your functions into smaller, more modular components to improve readability and maintainability."

Unactionable Feedback:
❌ "You’re not using best practices here."
Actionable Feedback:
✔️ "Consider using const or let instead of var for variable declarations to avoid potential scoping issues. This can help make your code more predictable."

Unactionable Feedback:
❌ "The formatting is wrong."
Actionable Feedback:
✔️ "It seems like the indentation is inconsistent. Following the style guide (e.g., using 2 spaces for indentation) could help make the code more consistent and easier to maintain."

That said this is meant to be an example, do we want to hold examples to this level of scrutiny? Is there a concern that someone will reference this example in their PR when we ask if they tested their code? It just seems like a lot of weight to place on an example.

[nkolev92]

I feel like the some of the examples in that list are really wordy. You can be nice without writing fluff.

That being said we're not adding those examples, but a different one.
I think it makes sense that the examples here are something universally agreed upon.
When using strong wording in guidelines like do this and don't do that, people are likely to take it very seriously.

I think it's worth coming up with a different example than the current one, one that doesn't put heavy burden on the reivewer.

[jgonz120]

Removed the example and asked chat gpt to give me some bullet points on providing actionable feedback.

[nkolev92]

I feel like the some of the examples in that list are really wordy. You can be nice without writing fluff.

That being said we're not adding those examples, but a different one.
I think it makes sense that the examples here are something universally agreed upon.
When using strong wording in guidelines like do this and don't do that, people are likely to take it very seriously.

I think it's worth coming up with a different example than the current one, one that doesn't put heavy burden on the reivewer.

Pull Request Overview

This PR restructures the workflow guidelines documentation to improve clarity and organization for pull requests and code reviews.

• Updated section headings to better represent document topics
• Revised guidelines for requesting and providing pull request feedback
• Enhanced instructions on merging PRs and branch strategies