Initial contributor activity data collection pipeline - #1
Conversation
…coverage - Make weekly activity synchronization idempotent, querying week-to-date - Deduplicate PR review credits per merged pull request - Fix ISO 8601 week number and year-boundary calculations - Prune 52-week rolling window based on filename ISO week rather than runner mtime - Eliminate bot false positives for human usernames ending with bot - Support ORG_READ_TOKEN in CI and client injection for unit tests - Sort contributor keys alphabetically in summary and weekly files - Add analysis_options.yaml and dart format check in validate.yml - Add unit tests for ISO weeks, sync logic, data integrity, and GitHubClient
| if (lower.endsWith('[bot]') || | ||
| lower.endsWith('-bot') || | ||
| lower.endsWith('_bot') || | ||
| lower.contains('autoroll') || | ||
| lower.contains('robot')) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This might exclude contributions from someone whose login happens to contain robot. Is the knownBots list enough? (The [bot] check seems reasonable though since that'll never match a user login!)
There was a problem hiding this comment.
Yeah, I feel like I should remove -bot and _bot in addition to robot. I am thinking some one like bell_bottom would get caught, or like cheers-bottoms-up, or aRbitraRy-botSomEthing. 😜
Bots should be part of the Robots Github team, but I am sure there are some gaps there. I can iterate on this as well as we 'work the ladder'.
There was a problem hiding this comment.
Err no, I confused myself. It's only checking endsWith on those ones, so maybe ok. I will leave them. I was still looking at the contains('robot')
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
Looks good to me with some minor nitpicks!
…put validation
- Handle deleted account ghost separately from isBot via isGhost and isExcludedAccount
- Remove lower.contains('robot') to prevent false-positives for human usernames
- Implement rate limit retry handling with Retry-After header and backoff in GitHubClient
- Validate GitHub usernames in user activity queries to protect against query injection
- Add unit tests for ghost handling, robot usernames, rate limit retries, and username validation
| ### Exclusions and filtering | ||
|
|
||
| - Accounts in the `@flutter/robots` GitHub team and known automation usernames defined in `tool/bots.dart` are excluded. | ||
| - Members of `@flutter/googlers` and `@flutter/partners` are excluded to conserve GitHub API rate limits. |
There was a problem hiding this comment.
I see ex-Googlers in the data. Maybe it's not worth the hassle of excluding them though, since the problem will resolve itself after 52 weeks.
There was a problem hiding this comment.
Yeah, I do have some special handling coming in the full implementation. I'll have a doc out for review on that soon.
| partners = await client.getTeamMembers('flutter', 'partners'); | ||
| await loadRobotsTeam(client); | ||
| } catch (_) { | ||
| // Elevated team read permissions may not be available with default GITHUB_TOKEN |
There was a problem hiding this comment.
Nit: Should anything be logged in this case? And/or should you only catch the specific type of error that the comment suggests here?
There was a problem hiding this comment.
Good call - updated all catch (_) where we don't want to throw to add logging. 👍
| ======================= | ||
|
|
||
| _tl;dr: join [Discord](./docs/contributing/Chat.md), be [courteous](CODE_OF_CONDUCT.md), follow the steps below to set up a development environment; if you stick around and contribute, you can [join the team](./docs/contributing/Contributor-access.md) and get commit access._ | ||
| _tl;dr: join [Discord](https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md), be [courteous](https://github.com/flutter/flutter/blob/main/CODE_OF_CONDUCT.md), follow the steps below to set up a development environment; if you stick around and contribute, you can [join the team](https://github.com/flutter/flutter/blob/main/docs/contributing/Contributor-access.md) and get commit access._ |
There was a problem hiding this comment.
I guess this was copied from flutter/flutter and the links didn't work?
There was a problem hiding this comment.
Yup. Kevin originally just copied the file over from flutter/flutter when the repo was set up, so the links were all relative.
3 participants
Adds workflows for collecting contributor data on a rolling basis. This will make quarterly analysis much much easier.
Part of flutter/flutter#186415
How to review this PR:
Everything under data/ is a year's worth of contributor data. You can skim it, but it is all generated by the other contents of this PR, where the review should focus. :)
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.