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

ADO Extension: PR comments are missing 2 cases #959

Closed
DaveTryon opened this issue Nov 9, 2021 · 2 comments
Closed

ADO Extension: PR comments are missing 2 cases #959

DaveTryon opened this issue Nov 9, 2021 · 2 comments
Labels
status: resolved This issue has been merged into main and deployed to canary.

Comments

@DaveTryon
Copy link
Contributor

DaveTryon commented Nov 9, 2021

We've identified the 2 following cases that are poorly represented in the PR comments that are created by ADO extension version 1.1.1:

Scenario 1: PR corrects some (but not all) violations

This occurs when:

  1. A baseline file exists
  2. The PR addresses some (but not all) of the violations in the baseline
  3. No new violations are introduced

The current code generates the "no new failures" PR message, which makes no mention of changes or updating the baseline file. A more appropriate PR comment would include verbiage about updating the baseline file, as well as a summary of what violations have been removed. These comments should help the PR author understand if the actual results of the PR match the anticipated results of the PR.

Scenario 2: PR modifies some violations without fixing them

This occurs when:

  1. A baseline file exists
  2. The PR includes a change that removes some existing violations but creates new ones. Since violations are specific to both the URL and HTML, this could happen if the HTML around a violation changes or if a page containing violations gets a different URL.

The current code generates the "new failures found" PR message, with no comment about the comments that have been fixed. It tells the user to update the PR, but makes no mention of the violations that no longer exist.

@ghost ghost added the status: new This issue is new and requires triage by DRI. label Nov 9, 2021
@ghost ghost assigned DaveTryon Nov 9, 2021
@DaveTryon DaveTryon added status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. and removed status: new This issue is new and requires triage by DRI. labels Nov 9, 2021
@ghost ghost assigned asksep Nov 9, 2021
@ghost
Copy link

ghost commented Nov 9, 2021

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@DaveTryon DaveTryon removed their assignment Nov 9, 2021
@asksep asksep added status: ready for work This issue is ready to be worked on. and removed status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. labels Dec 2, 2021
@DaveTryon DaveTryon assigned DaveTryon and unassigned asksep Dec 2, 2021
@DaveTryon DaveTryon added status: active This issue is currently being worked on by someone. and removed status: ready for work This issue is ready to be worked on. labels Dec 2, 2021
@DaveTryon
Copy link
Contributor Author

Confirmed fixed in Canary build 0.1.121

@DaveTryon DaveTryon added status: resolved This issue has been merged into main and deployed to canary. and removed status: active This issue is currently being worked on by someone. labels Dec 3, 2021
@DaveTryon DaveTryon removed their assignment Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: resolved This issue has been merged into main and deployed to canary.
Projects
None yet
Development

No branches or pull requests

2 participants