| 09:09 | <keithamus> | Is there something we can do about the pr-preview[bot] using stale data to overwrite issue comments. It makes it quite error prone to edit PR descriptions |
| 09:10 | <keithamus> | In the image above you can see i added a webkit bug for this PR, but the pr-previewbot updated the preview but in doing so used state comment data and undid the change, removing the webkit bug. |
| 09:17 | <annevk> | We have instructions for how to deal with this in the PR template as I mentioned. The alternative would be attempting to patch https://github.com/tobie/pr-preview I suppose. I'm not entirely sure how that would work as I think the comment has to be submitted as a whole to GitHub so there would always be a bit of a race. |
| 09:18 | <annevk> | (And you could imagine alternatives such as PR Preview always creating its own comment, but that adds noise for everyone that's not really desirable.) |
| 09:45 | <annevk> | Ideally GitHub would have some way for a bot to annotate issues/PRs with Markdown in a way that doesn't result in notifications, but we lost our resident GitHub person. 😀 |
| 09:47 | <keithamus> | I think typically bots add a comment to the timeline. There's also the "deployments" feature which was theoretically designed to solve this kind of issue - it adds a new timeline entry. Both of these have quite large trade-offs compared to updating OP though. |
| 09:47 | <Ms2ger> | If "minimal notifications" is also acceptable, we could have a single comment that the bot updates as needed |
| 09:49 | <annevk> | WPT used to have a single comment and that was a huge source of frustration. At least for Domenic and I. |
| 09:50 | <keithamus> | https://github.com/tobie/pr-preview/pull/163 hopefully this goes some of the way to minimising the issue at least. |
| 12:08 | <farre> | Noam Rosenthal: hey, I'm trying to figure out a buggy navigation api test case. I'm looking at navigate-event/navigate-navigation-navigate.html, and tracing it, I can only find that this should fire a navigate event with navigationType == "push". It does so in gecko, but chromium disagrees |
| 12:09 | <farre> | (and I got extra eyes from smaug, so I don't think I'm missing anything) |
| 12:11 | <smaug> | location.href setter doesn't behave like navigate() when it comes to 'completely loaded' checks. |
| 12:45 | <Noam Rosenthal> | Taking a look. |
| 12:46 | <annevk> | zcorpan: how do you want to handle that reformat thing? Should I rubber stamp it? Will a follow-up Meta commit ensure blame is not impacted? |
| 12:47 | <annevk> | Also not sure if jmdyck wants to take a look before it happens. |
| 12:57 | <Noam Rosenthal> | farre: yes it seems like chromium follows the location.href behavior of setting the history handling to replace when not completely loaded. However that's not in the spec. |
| 12:57 | <farre> | ok, cool. In an upcoming patch I'm updating the test to reflect spec |
| 12:59 | <jmdyck> | I'm still looking at the reformatting PR, I'm just delayed because the merge of the algorithm PR caused some re-org of my code |
| 13:44 | <zcorpan> | annevk: I think I'd like to add the tool to CI so that we can enforce it for PRs. Maybe integrate with html-build |
| 13:47 | <zcorpan> | annevk: For blame, one needs to specify to skip that commit manually, right? |
| 13:48 | <annevk> | zcorpan: https://docs.github.com/en/repositories/working-with-files/using-files/viewing-and-understanding-files#ignore-commits-in-the-blame-view |
| 13:49 | <zcorpan> | Ah, yeah can do that as a followup |
| 13:49 | <annevk> | I'm hoping that Searchfox plays ball, maybe asuth can confirm. |
| 14:34 | <jmdyck> | Is there interest in having a more comprehensive formatter for the HTML spec, that enforces more style guidelines (think rustmt or gofmt)? |
| 14:41 | <annevk> | jmdyck: yes, definitely! That would make life easier for reviewers and contributors. |
| 22:17 | <mfreed> | Hi all, just a friendly reminder to post any discussion topics for this Thursday's joint CSSWG/WHATWG/OpenUI task force meeting to the meeting agenda issue: https://github.com/whatwg/html/issues/11670 |