Upload much more data in status reports #116
Conversation
robertbrignull
force-pushed
the
status_reports_v2
branch
from
7754317 to
b2c89a2
Compare
| async function sendCompletedStatusReport( | ||
| startedAt: Date, | ||
| allLanguages: string[], | ||
| failingLanguage?: string, |
There was a problem hiding this comment.
Should this be an array if there are multiple language failures? Or are we only able to record the first language failure?
There was a problem hiding this comment.
My reasoning is, if one language fails, why would you bother building more? I think we will always want to abort after the first one.
There was a problem hiding this comment.
Makes sense. I'm guessing that the autobuilding is happening serially. I had thought that it was in parallel, in which case we might have multiple failures.
And actually, looking at the source, it seems like the action only ever builds a single language, no matter how many languages are available.
There was a problem hiding this comment.
Yes, you're right we currently only build one at a time. These status reports are trying to account for the plan that we will likely want to build multiple at some point. However they'll still be done in series because this is all happening on one machine, so I think we'll still only want to report one failure.
There was a problem hiding this comment.
OK. That makes sense and so having a single failing language is the way to go.
robertbrignull
force-pushed
the
status_reports_v2
branch
from
b2c89a2 to
9b2fc80
Compare
robertbrignull
force-pushed
the
status_reports_v2
branch
from
9b2fc80 to
87758a1
Compare
|
Now the server side has been updated this is looking good and everything is passing as expected. I'm not sure who to assign to. Perhaps @sampart or @chrisgavin, or @marcogario if you're willing as you've done the other parts of this change. |
chrisgavin
left a comment
chrisgavin
left a comment
There was a problem hiding this comment.
👍 LGTM and worked on a test repository.
| core.endGroup(); | ||
|
|
||
| } catch (e) { | ||
| // For now the fields about query performance are not populated |
There was a problem hiding this comment.
Would we only populate these in the case of a failure or always? They seem like they might be useful even if everything succeeded.
There was a problem hiding this comment.
The plan was to do them always. My hestitation in doing them in this was just that it'll require a few extra changes, so I wanted to do that in a second followup PR.
Uploads much more data in the status reports and makes the data much more specific to what was done in each action.
This follows the schema discussed previously. The only bit I don't implement in this PR is determining the performance of queries in each language and splitting default and custom queries. I could do but it would be another large change so I'm thinking it's best to split it up into a different PR to come later this week or next week.
This will fail until the github.com API endpoint has been updated to accept this kind of status report.
Merge / deployment checklist