feat: provide repository contributors insights #30

cecobask · 2023-08-20T18:44:03Z

Description

This pull request adds the feature to provide repository contributors insights.

What type of PR is this? (check all applicable)

Related Tickets & Documents

Closes #5

Mobile & Desktop Screenshots/Recordings

File contents:

$ cat repos.yaml

- https://github.com/kubernetes/kubernetes
- https://github.com/cli/cli

Usage with `--file` / `-f` flag

$ pizza insights contributors --file repos.yaml

┏━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Repository URL      ┃ https://github.com/cli/cli ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ New contributors    ┃ 25                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Recent contributors ┃ 30                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Churn contributors  ┃ 40                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Repeat contributors ┃ 5                          ┃
┗━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
┏━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Repository URL      ┃ https://github.com/kubernetes/kubernetes ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ New contributors    ┃ 158                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Recent contributors ┃ 280                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Churn contributors  ┃ 197                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Repeat contributors ┃ 122                                      ┃
┗━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

Usage with `--repos` / `-r` flag

$ pizza insights contributors --repos "https://github.com/kubernetes/kubernetes,https://github.com/cli/cli"

┏━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Repository URL      ┃ https://github.com/cli/cli ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ New contributors    ┃ 25                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Recent contributors ┃ 30                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Churn contributors  ┃ 40                         ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Repeat contributors ┃ 5                          ┃
┗━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
┏━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Repository URL      ┃ https://github.com/kubernetes/kubernetes ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ New contributors    ┃ 158                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Recent contributors ┃ 280                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Churn contributors  ┃ 197                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Repeat contributors ┃ 122                                      ┃
┗━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

Ignore non-existent and non-indexed repositories

$ pizza insights contributors --repos "https://github.com/fake-owner/fake-repo,https://github.com/kubernetes/kubernetes"

ignoring repository issue: repository https://github.com/fake-owner/fake-repo is either non-existent or has not been indexed yet
┏━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Repository URL      ┃ https://github.com/kubernetes/kubernetes ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ New contributors    ┃ 158                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Recent contributors ┃ 280                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Churn contributors  ┃ 197                                      ┃
┣━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
┃ Repeat contributors ┃ 122                                      ┃
┗━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

Error when neither `--file` or `--repos` flag was provided

$ pizza insights contributors

Error: must pass either --file or --repos flag
Usage:
  pizza insights contributors [flags]

Flags:
  -f, --file string     Path to yaml file containing an array of git repository urls
  -h, --help            help for contributors
  -p, --period int      Number of days, used for query filtering (default 30)
  -r, --repos strings   Comma-separated array of git repository urls

Global Flags:
      --beta              Shorthand for using the beta OpenSauced API endpoint ("https://beta.api.opensauced.pizza/v1"). supercedes the '--endpoint' flag
  -e, --endpoint string   The API endpoint to send requests to (default "https://api.opensauced.pizza/v1")

2023/08/20 19:36:16 must pass either --file or --repos flag

Error when both `--file` and `--repos` flags were provided

$ pizza insights contributors --file repos.yaml --repos "https://github.com/kubernetes/kubernetes,https://github.com/cli/cli"

Error: if any flags in the group [file repos] are set none of the others can be; [file repos] were all set
Usage:
  pizza insights contributors [flags]

Flags:
  -f, --file string     Path to yaml file containing an array of git repository urls
  -h, --help            help for contributors
  -p, --period int      Number of days, used for query filtering (default 30)
  -r, --repos strings   Comma-separated array of git repository urls

Global Flags:
      --beta              Shorthand for using the beta OpenSauced API endpoint ("https://beta.api.opensauced.pizza/v1"). supercedes the '--endpoint' flag
  -e, --endpoint string   The API endpoint to send requests to (default "https://api.opensauced.pizza/v1")

2023/08/20 19:36:42 if any flags in the group [file repos] are set none of the others can be; [file repos] were all set

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help

Added to documentation?

jpmcb

I'm realizing this has quite abit of overlap with #24.

We'll want to align the user experience so that there is a clear story for CLI users with each command

This will also be blocked on us getting an API client for Go: #28

.gitignore

README.md

cmd/bake/bake.go

pkg/api/constants.go

pkg/api/client.go

go.mod

cmd/root/root.go

cmd/insights/contributors.go

bdougie · 2023-08-21T22:33:07Z

I'm realizing this has quite abit of overlap with #24.

We'll want to align the user experience so that there is a clear story for CLI users with each command

I see the difference being the interactivity you get from the show. I didn't want to increase scope on #24 while we explore the basic interaction, but think we can do some more high level insights from commits and prs as well.

jpmcb · 2023-08-22T17:35:51Z

@cecobask - the Go API client has landed ✨ https://github.com/open-sauced/go-api !!!! Are you able to give that a try? It likely has some sharp edges so please open issues in that repo as the need arises!

jpmcb · 2023-08-22T21:57:28Z

Can you change this PR's target branch to be beta? 👀 We recently changed the default branch to beta in support of using the releaser action

For further reading:

Feature: ⚠️ ⚠️ ⚠️ Cut beta branch and make it the default branch #34

cecobask · 2023-08-23T10:48:15Z

@jpmcb, Done 👍🏻 Another review session is in order 😄

Refactored the codebase to make API calls using the go client where possible, the only exception being auth and repo-query commands
Consolidated duplicated logic and constants
Configured gci linter with default settings
Added concurrency to the pizza bake command
Updated pizza insights contributors to use https://github.com/charmbracelet/bubbles#table for outputting results

Until open-sauced/go-api#2 is resolved, pizza bake commands will fail with 400 Bad Request.

cecobask · 2023-08-26T17:03:36Z

The problems with open-sauced/go-api were resolved.
I believe there are no more changes to be added here.
@jpmcb, can you please take a look when you get a chance?

main.go

cecobask · 2023-08-26T17:10:15Z

cmd/root/root.go

+			}
+			return nil
+		},
+		SilenceUsage: true,


Every time a command returned an error, the help output was also printed, together with the error.
I think this is cleaner. The help output will still be shown in case the user entered invalid input for commands or passed --help flag.

Every time a command returned an error, the help output was also printed

We should still dump the help message when an error is returned since it's usually due to some misconfiguration of flags or values. The standard approach in Cobra CLIs is to print the help message along with the error so the user can have a quick loop to correcting it.

❯ ./build/pizza insights contributors Error: must pass either file or repos flag

Hmmm I suppose I could be convinced that this is cleaner and easier to grok. What do others think?

I think we still want some sort of user feedback. For example, in the github CLI, they return some helpful list of available commands:

❯ gh pr woof unknown command "woof" for "gh pr" Usage: gh pr <command> [flags] Available commands: checkout checks close comment create diff edit list lock merge ready reopen review status unlock view

So, for now, until we figure out a better strategy, let's just dump the help message for that command in order to provide some fast feedback for the user.

cmd/auth/auth.go

cmd/bake/bake.go

cmd/insights/contributors.go

cecobask

Thank you for the feedback, @jpmcb!
I've updated the code based on your suggestions.
Please, review at your convenience 👍🏻

cmd/bake/bake.go

pkg/api/constants.go

jpmcb · 2023-08-31T16:19:37Z

Thank you for the feedback, @jpmcb!

Happy to provide the feedback! This is looking good, great work. Huge lift. This is on my docket to dive back into today!

jpmcb

This is getting very close!!!! Nice work!!!

cmd/bake/bake.go

jpmcb · 2023-08-31T20:39:22Z

cmd/root/root.go

+			}
+			return nil
+		},
+		SilenceUsage: true,


I think we still want some sort of user feedback. For example, in the github CLI, they return some helpful list of available commands:

❯ gh pr woof unknown command "woof" for "gh pr" Usage: gh pr <command> [flags] Available commands: checkout checks close comment create diff edit list lock merge ready reopen review status unlock view

So, for now, until we figure out a better strategy, let's just dump the help message for that command in order to provide some fast feedback for the user.

cmd/bake/bake.go

pkg/utils/arguments.go

cmd/insights/contributors.go

cecobask · 2023-09-02T16:43:27Z

This is getting very close!!!! Nice work!!!

Thanks, @jpmcb! I've updated the branch with the latest changes 👍🏻
Let me know if something else needs to be done here.

jpmcb

Great work!! Just one small nit on a regression I noticed in using a custom endpoint for the pizza service (which I typically do for testing local changes). This might be an issue with how the client generates it's endpoints. Otherwise, let's get this in! 💪🏼

cmd/bake/bake.go

bdougie

I have been watching this quietly in the background and can say it works as expected for me.

re: localhost. It might be worth setting up some follow up issues to identify things we can fix and enhance after this is merged.

Something top of mind is making these contributor insights actionable with some context on % and story telling.

Perhaps we get @isabensusan to assist in adding this to the design backlog to assist in representing this in the UI.

## [1.0.0-beta.4](v1.0.0-beta.3...v1.0.0-beta.4) (2023-09-06) ### 🍕 Features * provide repository contributors insights ([#30](#30)) ([d16091f](d16091f))

## 1.0.0 (2023-10-11) ### 🤖 Build System * sematic bin release, npm ([7b4607e](7b4607e)) ### 🔁 Continuous Integration * Update @open-sauced/[email protected] and compliance.yaml ([#33](#33)) ([146b6b7](146b6b7)) ### 🐛 Bug Fixes * Uses correct generated token when checking out cli repo in release ([#44](#44)) ([1e0c9f1](1e0c9f1)) ### 🍕 Features * Add install instructions and script for pizza CLI ([#26](#26)) ([421a429](421a429)) * Add posthog telemetry integration ([#37](#37)) ([9829f49](9829f49)) * cli auth ([#21](#21)) ([34728fb](34728fb)) * GitHub action to build and upload Go artifacts after release created ([#22](#22)) ([ad187a9](ad187a9)) * Http Client for accessing OpenSauced API client ([#23](#23)) ([ec2b357](ec2b357)) * Leverage the GITHUB_APP_TOKEN for releases ([#32](#32)) ([e0a25e0](e0a25e0)) * npm i -g pizza ([73291d1](73291d1)) * Pizza show ([#24](#24)) ([72f21ce](72f21ce)) * provide repository contributors insights ([#30](#30)) ([d16091f](d16091f)) * provide repository insights ([#38](#38)) ([dc148d6](dc148d6)) * repo-query support ([199cfd7](199cfd7)) * update bin name release.yaml ([6b21cb8](6b21cb8)) * Version command for CLI based on release builds ([#36](#36)) ([9f3eedc](9f3eedc))

cecobask force-pushed the contributor-insights branch 2 times, most recently from c06f01e to 6e3ea56 Compare August 21, 2023 08:22

jpmcb requested changes Aug 21, 2023

View reviewed changes

k1nho reviewed Aug 21, 2023

View reviewed changes

cmd/insights/contributors.go Outdated Show resolved Hide resolved

cecobask force-pushed the contributor-insights branch from e7b0bb5 to 0c02404 Compare August 21, 2023 21:14

cecobask requested a review from jpmcb August 21, 2023 21:14

cecobask changed the base branch from main to beta August 23, 2023 10:16

cecobask force-pushed the contributor-insights branch 2 times, most recently from fabf3f5 to eb70fb1 Compare August 23, 2023 10:26

cecobask force-pushed the contributor-insights branch from eb70fb1 to bf539a5 Compare August 26, 2023 17:01

cecobask commented Aug 26, 2023

View reviewed changes

cecobask force-pushed the contributor-insights branch from bf539a5 to 71be66d Compare August 26, 2023 17:17

jpmcb requested changes Aug 28, 2023

View reviewed changes

jpmcb reviewed Aug 28, 2023

View reviewed changes

cmd/insights/contributors.go Outdated Show resolved Hide resolved

cecobask commented Aug 29, 2023

View reviewed changes

cmd/bake/bake.go Outdated Show resolved Hide resolved

pkg/api/constants.go Outdated Show resolved Hide resolved

cecobask requested a review from jpmcb August 29, 2023 12:03

cecobask force-pushed the contributor-insights branch 3 times, most recently from bad32a5 to f20fd71 Compare August 31, 2023 16:14

jpmcb reviewed Aug 31, 2023

View reviewed changes

feat: provide repository contributors insights

6e95a76

cecobask force-pushed the contributor-insights branch from f20fd71 to 6e95a76 Compare September 2, 2023 16:41

cecobask requested a review from jpmcb September 2, 2023 16:43

jpmcb approved these changes Sep 5, 2023

View reviewed changes

cmd/bake/bake.go Show resolved Hide resolved

bdougie approved these changes Sep 6, 2023

View reviewed changes

jpmcb merged commit d16091f into open-sauced:beta Sep 6, 2023

cecobask deleted the contributor-insights branch September 6, 2023 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: provide repository contributors insights #30

feat: provide repository contributors insights #30

cecobask commented Aug 20, 2023 •

edited

Loading

jpmcb left a comment

bdougie commented Aug 21, 2023

jpmcb commented Aug 22, 2023

jpmcb commented Aug 22, 2023

cecobask commented Aug 23, 2023

cecobask commented Aug 26, 2023

cecobask Aug 26, 2023

jpmcb Aug 28, 2023

jpmcb Aug 28, 2023

jpmcb Aug 31, 2023

cecobask left a comment

jpmcb commented Aug 31, 2023

jpmcb left a comment

jpmcb Aug 31, 2023

cecobask commented Sep 2, 2023

jpmcb left a comment

bdougie left a comment

feat: provide repository contributors insights #30

feat: provide repository contributors insights #30

Conversation

cecobask commented Aug 20, 2023 • edited Loading

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

File contents:

Usage with --file / -f flag

Usage with --repos / -r flag

Ignore non-existent and non-indexed repositories

Error when neither --file or --repos flag was provided

Error when both --file and --repos flags were provided

Added tests?

Added to documentation?

jpmcb left a comment

Choose a reason for hiding this comment

bdougie commented Aug 21, 2023

jpmcb commented Aug 22, 2023

jpmcb commented Aug 22, 2023

cecobask commented Aug 23, 2023

cecobask commented Aug 26, 2023

cecobask Aug 26, 2023

Choose a reason for hiding this comment

jpmcb Aug 28, 2023

Choose a reason for hiding this comment

jpmcb Aug 28, 2023

Choose a reason for hiding this comment

jpmcb Aug 31, 2023

Choose a reason for hiding this comment

cecobask left a comment

Choose a reason for hiding this comment

jpmcb commented Aug 31, 2023

jpmcb left a comment

Choose a reason for hiding this comment

jpmcb Aug 31, 2023

Choose a reason for hiding this comment

cecobask commented Sep 2, 2023

jpmcb left a comment

Choose a reason for hiding this comment

bdougie left a comment

Choose a reason for hiding this comment

cecobask commented Aug 20, 2023 •

edited

Loading

Usage with `--file` / `-f` flag

Usage with `--repos` / `-r` flag

Error when neither `--file` or `--repos` flag was provided

Error when both `--file` and `--repos` flags were provided