Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement pizza offboard command #207

Open
wants to merge 9 commits into
base: beta
Choose a base branch
from

Conversation

zeucapua
Copy link
Contributor

@zeucapua zeucapua commented Sep 24, 2024

Description

Implements a new pizza offboard command that allows the user to offboard someone from the .sauced.yaml config and CODEOWNERS files by their attributed name or email.

Related Tickets & Documents

Closes #117

Mobile & Desktop Screenshots/Recordings

Steps to QA

  1. Build the program
  2. Ensure you have the .sauced.yaml and CODEOWNERS files
  3. Run pizza offboard <usernames/emails> with the user(s) you want to offboard
  1. Use the --path (-p) flag to specify what repository you are updating
  2. Use the --config (-c) global flag to specify the config
  3. After running, check the config and owners files to ensure that any attributions to the offboarded users are removed

Tier (staff will fill in)

  • Tier 1
  • Tier 2
  • Tier 3
  • Tier 4

[optional] What gif best describes this PR or how it makes you feel?

a-group-of-people-in-life-jackets-are-standing-on-a-floating-platform-in-the-water

@zeucapua zeucapua marked this pull request as draft September 24, 2024 17:42
@zeucapua zeucapua marked this pull request as ready for review September 25, 2024 23:05
@zeucapua zeucapua requested a review from a team September 26, 2024 17:28
Copy link
Member

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but it wasn't clear to me that I had to put a space between people or emails I wanted to offboard. I tried using a comma initially.

It'd also be nice to detect the .sauced.yaml like we do with generate.

A future feature could be loading a CSV of people to offboard, but now I'm scope creeping. 😅

Comment on lines +26 to +27
const offboardLongDesc string = `[WIP] Removes a user from the \".sauced.yaml\" config and \"CODEOWNERS\" files.
Requires the user's name OR email.`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of [WIP], could be "Caution: Experimental command."

Could also include examples like we do elsewhere

opts.ttyDisabled, _ = cmd.Flags().GetBool("tty-disable")
opts.configPath, _ = cmd.Flags().GetString("config")

opts.path, _ = cmd.Flags().GetString("path")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag likely needs to get marked as required. Otherwise, it attempts to use the default value:

❯ pizza offboard jpmcb
Error: error generating config file: error creating  file: open : no such file or directory


return nil
},
RunE: func(cmd *cobra.Command, _ []string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also initiate a Posthog client / runner for use with this command.

Comment on lines +90 to +93
err = generateConfigFile(opts.configPath, attributions)
if err != nil {
return fmt.Errorf("error generating config file: %v", err)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something here seems to be broken since I get errors when it tries to re-generate the config if I don't provide a config:

pizza offboard jpmcb -p ./

I'd expect it'd look in the directory provided by -p to actually find a .sauced.yaml or fallback to the one in the user's home directory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Feedback - Offboarding experience with pizza clean or pizza offboard
3 participants