Filter inherited stats by ownership#155
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new opt-in filtering mode to exclude “inherited” repositories (forks and repos not owned by the authenticated user) from aggregated statistics, addressing inflated totals reported in #152.
Changes:
- Extends repository metadata collection to include fork status and owner login.
- Adds
exclude_inheritedfiltering during aggregation to only count user-owned, non-fork repos. - Enables the new env flag in the default GitHub Actions workflow.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/statistics.zig |
Fetches and stores isFork + owner.login on each repo; updates deinit to free the new field. |
src/main.zig |
Adds exclude_inherited arg and filters repositories during aggregation based on ownership. |
.github/workflows/main.yml |
Sets EXCLUDE_INHERITED in the workflow environment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fork: bool, | ||
| owner_login: []const u8, | ||
|
|
||
| pub fn deinit(self: @This(), allocator: std.mem.Allocator) void { | ||
| allocator.free(self.name); | ||
| allocator.free(self.owner_login); |
There was a problem hiding this comment.
Repository gained new required fields (fork, owner_login). Statistics.initFromJson uses std.json.parseFromSliceLeaky(Statistics, ...), so older JSON files that don't include these fields will now fail to parse. Consider making these fields optional (or providing safe defaults) and updating deinit/aggregation logic accordingly so JSON input remains backwards-compatible.
| fork: bool, | |
| owner_login: []const u8, | |
| pub fn deinit(self: @This(), allocator: std.mem.Allocator) void { | |
| allocator.free(self.name); | |
| allocator.free(self.owner_login); | |
| fork: bool = false, | |
| owner_login: ?[]const u8 = null, | |
| pub fn deinit(self: @This(), allocator: std.mem.Allocator) void { | |
| allocator.free(self.name); | |
| if (self.owner_login) |owner_login| { | |
| allocator.free(owner_login); | |
| } |
| EXCLUDE_REPOS: ${{ secrets.EXCLUDE_REPOS }} | ||
| EXCLUDE_LANGS: ${{ secrets.EXCLUDE_LANGS }} | ||
| EXCLUDE_PRIVATE: "false" | ||
| EXCLUDE_INHERITED: "true" |
There was a problem hiding this comment.
The PR description frames EXCLUDE_INHERITED as an opt-in flag, but the repo workflow sets EXCLUDE_INHERITED: "true" unconditionally, which changes the default generated stats behavior for this repo (and for anyone copying the workflow). Consider omitting this env var by default, or sourcing it from a secret/input and defaulting to "false".
There was a problem hiding this comment.
I actually wonder if the variable should be set by default to "true' in the code itself. The inflated statistics can be drastic with only a small contribution to a popular repository.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exclude_repos: ?[]const u8 = null, | ||
| exclude_langs: ?[]const u8 = null, | ||
| exclude_private: bool = false, | ||
| exclude_inherited: bool = true, |
There was a problem hiding this comment.
exclude_inherited defaults to true, but boolean CLI flags in this repo’s argparse implementation can only be set to true via --exclude-inherited (there’s no CLI way to pass false). With a true default, users can’t disable this behavior from the CLI, only via env (EXCLUDE_INHERITED=false). Consider defaulting this to false (opt-in) or extending the CLI parsing to support explicit boolean values / a --include-inherited override.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
+1 for this—it'll really be useful. I don't want to count myself toward the stats of a large repository where I've only made a few changes. @jstrieb It would be great if you could take a look at this |
|
It's true, but what if you want your contributions represented? Maybe filter repositories individually. As for view counts, they shouldn't be included anyway. |
|
I don't think a perfect solution exists. It's either less contributions reported than you have done, or all of them but inflated and they might not be representative anymore, depending on your involvement in upstream projects and their respective size and activity. In my case, the v2.0 reported numbers were so wildly unrealistic that it made the whole github-stats irrelevant. |
Adds an EXCLUDE_INHERITED flag that, when enabled, restricts aggregated stats (stars, forks, lines changed, views, repos) to repositories the authenticated user actually owns — i.e. not forks, and where the owner login matches the user.
Why: Contributed-to repositories and forks can inflate star/fork counts and language breakdowns, making the stats card misleading as a measure of a user's own output. This flag lets users opt into a stricter, ownership-based view.
Fixes #152