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

docs: add port review template & revamp submission guidelines #1937

Merged
merged 15 commits into from
Feb 27, 2023

Conversation

sgoudham
Copy link
Contributor

@sgoudham sgoudham commented Feb 22, 2023

Motivation

As we recently switched over to GitHub discussions for our port requests, we have noticed a slight inefficiency in the new process. Ports are sometimes fully themed and ready to review before a discussion has been created. This is, of course, completely okay but results in a wasted step of creating the discussion and transferring it to an issue instantly.

Proposal

Create two types of port requests...

  1. Port Requests (GitHub Discussions)
    • This is the same process we currently have for discussions. However, this will now be restricted to only WIP ports and suggestions/ideas for ports to be included within Catppuccin.
  2. Port Reviews (GitHub Issues)
    • New issue template on the repository itself, ports that are already themed and ready for review should be raised here instead of the GitHub discussion.

We hope that this isn't too confusing for our existing maintainers and new maintainers to come.

P.S Sorry for the reformatting changes 😅

TODO List before/during merging

  • Figure out the URL for port-review to reference within port-suggestion.md and port-creation.md
  • Rename port-requests to port-suggestions in GitHub discussions
  • Decide on a workflow visualization diagram to include in port-creation.md

@sgoudham sgoudham added the meta Org-wide matters label Feb 22, 2023
@sgoudham sgoudham requested a review from a team February 22, 2023 03:33
@ghostx31
Copy link
Member

Great PR Hammy! The document changes look good.

As for the URL for port-review, we can probably take some idea from catppuccin/cli which opens the issue template with the URL: https://github.com/catppuccin/cli/issues/new?assignees=&labels=bug&template=bug_report.md&title=.

@nekowinston nekowinston marked this pull request as ready for review February 22, 2023 09:11
Copy link
Contributor

@nekowinston nekowinston left a comment

Choose a reason for hiding this comment

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

Looks good, some quick thoughts since we're reworking this again.

Edit: didn't mean to mark this as ready, just a GitHub mobile moment.

.github/ISSUE_TEMPLATE/port-review.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/port-review.yml Outdated Show resolved Hide resolved
docs/port-creation.md Outdated Show resolved Hide resolved
@sgoudham sgoudham marked this pull request as draft February 22, 2023 10:08
@sgoudham
Copy link
Contributor Author

Changing this back to draft because I want to add more stuff aside from PR comments

@ghostx31 ghostx31 marked this pull request as ready for review February 22, 2023 10:32
@ghostx31 ghostx31 marked this pull request as draft February 22, 2023 10:35
@backwardspy
Copy link
Member

backwardspy commented Feb 22, 2023

@catppuccin/staff we have put together a couple of options for visualising the port creation workflow here and here. these are of course rough drafts at this stage.

interested in your thoughts on how useful this will be, and whether another tool might be better (excalidraw et al.) we started with mermaid for ease of maintenance, however this does place restrictions on styling & layout.

docs/port-creation.md Outdated Show resolved Hide resolved
- Add maintenance section
- Extend style guidelines
- Switch back to "Port Requests"
- Refactor "port-creation.md"
- Rename port suggestions to port requests
- Remove labels from port-review.yml
- Fix URL for port-review.yml
@sgoudham
Copy link
Contributor Author

sgoudham commented Feb 23, 2023

Tidied up the docs as per PR comments, curious about what people think of the new port-creation.md

Tried to get everything underneath the #Submission heading and link off to it. Worth mentioning that the visualisation diagram is still a WIP!

@alythical
Copy link
Member

I found the diagram by @nekowinston to be the easiest to follow because in the other two it didn't seem as clear where the workflow starts for people who already have completed themes that are ready for review.

@ghostx31
Copy link
Member

@backwardspy I found winston's diagram easier to follow. Maybe we might add it to port-creation.md?
Also, about port-creation.md:

@nullishamy
Copy link
Contributor

nullishamy commented Feb 23, 2023

It would be nice if staff-transferred discussions and 'port reviews' were standardised to be the same / similar. Would further reduce friction and confusion for people reading the issues.

Other than that, LGTM, good work!

@sgoudham
Copy link
Contributor Author

sgoudham commented Feb 23, 2023

Really great point you've raised there @nullishamy

In the short term, I think there'll be a burden on staff to ensure that transferred issues are reformatted into the port-review template. In the longer term, I'd love the existence of a GitHub bot to automate the manual processes... one day :P

@sgoudham
Copy link
Contributor Author

Just one thing that I noticed in your port workflow diagram, I was under the following impression

  1. Port Requests (GitHub Discussions)
    • This is the same process we currently have for discussions. However, this will now be restricted to only WIP ports and suggestions/ideas for ports to be included within Catppuccin.
  2. Port Reviews (GitHub Issues)
    • New issue template on the repository itself, ports that are already themed and ready for review should be raised here instead of the GitHub discussion.

The diagram seems to suggest that for port requests, we actually don't want WIP ports at all? Is that correct? @nekowinston

@backwardspy
Copy link
Member

backwardspy commented Feb 24, 2023

The diagram seems to suggest that for port requests, we actually don't want WIP ports at all? Is that correct? @nekowinston

this is tricky. there are really three cases, while the graph currently only shows two.

  1. you want a port, and you don't want to make it.
  2. you want a port, and you want to make it
  3. you've already made a port

1 and 3 send you down the right process, but 2 implies you should wait until you've made the port. i think we definitely want to encourage the creation of a discussion in either case. perhaps the question "are you willing to make it" should instead be "have you made it already"?

@nekowinston
Copy link
Contributor

The diagram seems to suggest that for port requests, we actually don't want WIP ports at all? Is that correct? @nekowinston

Ah no, sorry, that's not what I meant. I made the stateDiagram as a draft to show off the graph layout, so consider it a draft and not a finished graphic.

this is tricky. there are really three cases, while the graph currently only shows two.

stateDiagram-v2
  choice: Are you willing to make the port?
  choice --> author: Yes
  author: Work on a draft for a port
  author --> feedback_needed
  feedback_needed: Do you want feedback while porting?
  feedback_needed --> author_done : No
  feedback_needed --> request: Yes
  author_done: Once a port is finished, create an issue, requesting review from @staff
  choice --> request: No
  request: Open a Port Request discussion
  request --> author_done

Do we want something like this then?
Who do we want to create the issues from a discussion? Staff or the authors?

Here's how I understand what we're trying to achieve with rewording this, using @backwardspy's list:

  1. you want a port, and you don't want to make it.
  2. you want a port, and you want to make it
  3. you've already made a port
  1. Raise a discussion, for people to pick up
  2. Raise a discussion to talk about the details of making it easy to maintain, etc.
  3. Fast-tracking current org members so that they only have to create an issue

What we could also add (we're assuming that we're starting out at a port idea):

  • Pointing people who want to work on something, but don't have an idea, to the discussions for inspiration

@backwardspy
Copy link
Member

backwardspy commented Feb 24, 2023

it seems to me that by changing the question at the start, we can keep the flow simple while covering (i think!) all cases:

stateDiagram-v2
  idea: You have an idea for a port
  idea --> choice

  choice: Have you already completed a draft?
  choice --> draft_complete: Yes
  choice --> request: No

  request: Open a Port Request discussion
  request --> request_picked_up
  
  request_picked_up: You or somebody else works on a draft
  request_picked_up --> request_complete

  request_complete: The draft is completed to your liking
  request_complete --> draft_complete
  
  draft_complete: Create a Port Review issue
  draft_complete --> review

  review: Review period
  review --> port_adoption

  note right of review
    Community feedback
  end note

  port_adoption: Port adoption

thoughts?

@nekowinston
Copy link
Contributor

Yeah I think that would cover everything. Way easier to read than more branches in the graph.

docs/port-creation.md Outdated Show resolved Hide resolved
docs/port-creation.md Outdated Show resolved Hide resolved
@taka0o taka0o self-requested a review February 27, 2023 14:53
@sgoudham sgoudham marked this pull request as ready for review February 27, 2023 16:05
@sgoudham
Copy link
Contributor Author

Before I merge this in, are you folks happy if I transfer the existing draft-available issues into discussions?

@nekowinston @backwardspy @ghostx31 @taka0o

@ghostx31
Copy link
Member

Sure, it's a yes from me.

@nekowinston
Copy link
Contributor

Yeah, but you can skip #152, I'm planning to make a repo for it this week, I already have it locally.

@sgoudham
Copy link
Contributor Author

Drafts moved over, merging now. Thanks, everyone for the comments and feedback throughout this! ❤️

@sgoudham sgoudham merged commit 3153d07 into main Feb 27, 2023
@sgoudham sgoudham deleted the docs/port-review branch February 27, 2023 22:56
sgoudham added a commit to sgoudham/catglasscupofhetableppuccin that referenced this pull request Jun 13, 2023
…ccin#1937)

Co-authored-by: backwardspy <backwardspy@pigeon.life>
Co-authored-by: winston <hey@winston.sh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Org-wide matters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants