-
Notifications
You must be signed in to change notification settings - Fork 347
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
Infrastructure: Transition revision date generation for Coverage and Quality Report to build repository workflow #2976
Conversation
The ARIA Authoring Practices (APG) Task Force just discussed The full IRC log of that discussion<jugglinmike> Subtopic: Approach to keeping date correct on coverage report<jugglinmike> github: https://github.com//pull/2976 <jugglinmike> Matt_King: My understanding is that, if we were to take this approach and merge this, then every time we merge a pull request, there's going to be a commit for merging the pull request and there's going to be a subsequent commit for updating the date on the quality report <jugglinmike> howard-e: That's correct <jugglinmike> Matt_King: Could we change the script that generates the report so that the date that is in the report itself doesn't change? <jugglinmike> Matt_King: For example, lets say we took jongund's "radio" pull request here. Lets say it was March 26, so his commit is March 26. The quality report runs on March 26, and it doesn't use the date that it runs--it uses the date of last changed file. <jugglinmike> Matt_King: That way, if he ran the coverage report on April 1, then it will still keep March 26, because it only considers the state of the code <jugglinmike> howard-e: I think why I ended up doing like this--it was based on the comment that the date should reflect the moment it was merged into the "main" branch <Jem> "As a request of yesterday's APG meeting, this PR will capture any changes found in that coverage report's diff check and immediately push it to the main branch because the "last updated date" should reflect when the affecting PR is merged in." <jugglinmike> Matt_King: The only reason we want a date there is for us to be able to see that the report includes the latest changes in the repository <jugglinmike> Matt_King: I don't know if the date of the merge to "main" is that important <jugglinmike> Matt_King: I've been trying to keep the state of the history of the "main" branch as clean as possible <jugglinmike> Matt_King: Now, these commits are labeled "CHORE:", so we can filter them out. So maybe I shouldn't even care. <jugglinmike> Matt_King: But it seems like having the extra commits is just more to ignore <jugglinmike> howard-e: If the date that it is merged into "main" truly doesn't matter, then I can walk back that second "CHORE" commit. It wouldn't be a problem <jugglinmike> howard-e: My other suggestion was making a transform on the "build" repo. So that none of this date manipulation would happen here <jugglinmike> Matt_King: If we're working on pull requests that would cause changes to a report, we'll want to be able to look in the report itself and see that the report was updated <jugglinmike> howard-e: The date would still show up in the preview, even if it was part of a transform in the "build" repository <jugglinmike> Matt_King: So we would still have the "coverage and quality report" script and generation all done in the repository <jugglinmike> Matt_King: Because it's part of the content, I kind of like having it in the "content" repository, but I'm okay with either <jugglinmike> Jem: So the last-update date would be when? <jugglinmike> Matt_King: It would match the date in the footer <jugglinmike> s/in the footer/in the footer on example pages/ <jugglinmike> s/it would match/the "last update" date would match/ <jugglinmike> howard-e: If it's done from APG, the date would reflect the moment of the latest contribution. If it's done from the "build" repository, the date would reflect the moment of the merge <jugglinmike> Matt_King: You're right. It's probably better to do this as a preview, that way, the two kinds of dates that we're exposing on the site would match <Jem> https://www.w3.org/WAI/ARIA/apg/patterns/alert/examples/alert/ <jugglinmike> Matt_King: Why does "alert" say it's been updated in the past two months? <jugglinmike> howard-e: We updated "skipTo" across all the pages <jugglinmike> Matt_King: Ah. This is why we need that other feature we discussed--to make that date link to the list of relevant commits. That way, folks could understand what the date actually described <jugglinmike> Matt_King: I have a design issue for this somewhere. It kind of got de-prioritized <jugglinmike> Matt_King: We don't have any complaints recently, but there is an issue related to this in the backlog <jugglinmike> Matt_King: In the future, "skipTo" changes will not effect this date. That's a beautiful part of jongund's recent changes to skipTo--we'll never have to update the pages when "skipTo" changes |
…I build repo transform instead
Updated this PR's description and reverted changes that were no longer needed since the date update will now happen through the build repository's transform when w3c/wai-aria-practices#311 is included. The Coverage and Quality Reports page preview link now accurately shows that the page was last updated on February 20, 2024. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@mcking65 @a11ydoer w3c/wai-aria-practices#311 has now been merged, so the date that the coverage-and-quality-report.html file is last updated on the For this branch, the file was last updated on Apr 2. So I've gone ahead rebuilt this PR's generated link on Apr 3 and Apr 4 to confirm the proper date is still getting tracked. The preview of the Coverage and Quality Reports page still shows April 2, 2024 and will continue to when pushed to Also confirmed the above through a local build of the I believe this is now ready to be merged and will clear up instances of failing "Check examples/index.html" builds once branches are rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Howard. Looks good.
Following the merge of #2122 and the inclusion of a "Page last updated" field on the coverage and quality report page, there is a surety that the
examples.yml
'scoverage:
job will fail onmain
if a PR triggeringexamples.yml
is merged on a date after what has already been set in the PR.This is due to a failing diff check caused by the more recent date being generated by
npm run coverage-report
when the PR is merged into main: examples.yml#61-62As a request of a recent APG meeting, this PR will stop tracking the date change in this repository and instead depend on the wai-aria-practices build repository to include the date at the point when a link is generated. The latest date will be based on the most recent time that the coverage-and-quality-report.html file was updated on
main
.w3c/wai-aria-practices#311 will need to be approved and merged to further support this change.
WAI Preview Link (Last built on Thu, 04 Apr 2024 17:24:40 GMT).