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.

Sign up for GitHub

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

Jump to bottom

Improve the list guidelines and pull request guidelines (pull request template) #2984

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve the list guidelines and pull request guidelines (pull request template) #2984

wants to merge 3 commits into from
+384 −112

Conversation

wongyah
Copy link

@wongyah wongyah commented Feb 27, 2024 •
edited
Loading

Why I open the pull request

I read through the pull request template and other files before I started to create my list Awesome Technical Writing Learning. I stayed confused during my reading:

  • Why should I know how to write a pull request when I don't even have an awesome list? ("Requirements for your pull request" is the first section.)
  • Why I just know how to create the awesome list when I am ready to submit the pull request? ("Requirements for your Awesome list is the second section.)
  • Why I just know there are requirements for repository settings so late? (repository name, default branch name, and topics setting are not at the beginning of the file. So do requirements on other parts of the list.)

What I did

  • Separate the requirements for the awesome list from those for the pull request, resulting in two files named "awesome-list-guidelines.md" and "pull-request-guidelines.md".
  • Classify the requirements in different categories, such as repository settings, list profile, table of contents, list body, ect.
  • Create a section for each category.
  • Arrange the sections based on the journey of list creation and submission, so that the list owners can follow the guidelines to create their list.
  • Add a table of contents for each guidelines file, so that the list owners can place the relevant requirements immediately.

What is in question

There are several sentences in the pull request template I maybe didn't understand correctly. I added a comment for each of them, like this:

<!--! What do you mean by "Non-generated Markdown file in a GitHub repo."? Maybe I didn't get your point here. -->
> [!NOTE]
> Do not use generated Markdown files in the list repository.

<!--! Is it the function of Yeoman generator? I didn't understand the readme of Yeoman generator. -->
> [!TIP]
> You can use [Yeoman generator](https://github.com/dar5hak/generator-awesome-list) to create a new awesome list automatically.

If you like the style of the improved guidelines, please feel free to review and comment on how to make it better. Thanks!

@wongyah
Improve the contents in "pull request template":
8fecd0e 
1. Categorize the guidelines into sections within two files.
2. Simplify the instructions and description.
3. Remove the redundant information.
@wongyah
Refine the awesome list guidelines and the pull request guidelines.
1a9cc6b
@wongyah
Correct some typos and inconsistences in awesome list guidelines and …
80c3f73 
…pull request guidelines.
Repository owner deleted a comment from Kien329 Mar 5, 2024
rsamborski
rsamborski suggested changes Mar 26, 2024
View reviewed changes
awesome-list-guidelines.md
<!-- omit from toc -->
# Awesome List Guidelines

> [!NOTE]
Copy link
Contributor

Choose a reason for hiding this comment

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

There are 4 notes and 1 tip in this section. Please consider adding moving some of the content to generic text for ease of reading.

pull_request_template.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should preserve the template as checklist. I suggest linking to the guideline documents for more details, but IMHO a checklist has big value allowing people to quickly validate if they have completed all required steps.

@rsamborski rsamborski mentioned this pull request Mar 26, 2024
Merged
33 tasks
@sindresorhus
Copy link
Owner

Thank you for your contribution. However, the checklist format was intentionally chosen due to extensive experience showing that longer texts often go unread. The checklist encourages thorough review of each item, reducing the likelihood of important details being overlooked.

@sindresorhus
Copy link
Owner

Why should I know how to write a pull request when I don't even have an awesome list? ("Requirements for your pull request" is the first section.)

We could link directly to the correct section: https://github.com/sindresorhus/awesome/blob/main/pull_request_template.md#requirements-for-your-awesome-list

@sindresorhus
Copy link
Owner

I'm definitely open to improving things, but I prefer for the checklist format to stay for the text in the pull request guidelines file.

@wongyah
Copy link
Author

wongyah commented Apr 7, 2024 •
edited
Loading

Thank you for your contribution. However, the checklist format was intentionally chosen due to extensive experience showing that longer texts often go unread. The checklist encourages thorough review of each item, reducing the likelihood of important details being overlooked.

It is true that a checklist encourages people to review each item in it, but only when the checklist is simple and short.

A simple statistic

I did a simple statistic for all the pull requests in this repo during this year (before I send the PR) :

  • There are 17 pull requests in total, but only 7 pull requests (41%) shows that their list creators read most of the content in pull_request_template.md.
  • Among the 7 pull requests, 4 pull requests were send by one creator who applied 4 times to add the same list. That means, only 4 of 14 creators (28.6%) seem to read most of the content in pull_request_template.md.
  • Among the rest pull requests, 5 pull requests show their list creators didn't read the guidelines at all, 2 pull requests show their list creators may read a little of the guidelines but didn't read all of it for sure, 3 pull requests are spam.

My opinion on checklist

I fully agree with you that there should be a checklist for creators and reviewers to check and mark if the list is compliant to the guidelines. But it is better to separate it from the requirement details.

Here is an example:

- [ ] (Repository settings)[awesome_list_guidelines.md#repository-settings]
    - [ ] (Repository name)[awesome_list_guidelines.md#repository-name]
    - [ ] (Default branch)[awesome_list_guidelines.md#default-branch]
- [ ] (List profile)[awesome_list_guidelines.md#list-profile]
    - [ ] (List title)[awesome_list_guidelines.md#list-title]
    - [ ] (Awesome badge)[awesome_list_guidelines.md#awesome-badge]
    - [ ] (Short description)[awesome_list_guidelines.md#short-description]
    - [ ] (Logo and header image)[awesome_list_guidelines.md#logo-and-header-image]
- [ ] (Table of contents)[awesome_list_guidelines.md#table-of-contents]
...

The items are ordered according to the procedure of creating an awesome list, same to where they appear in awesome_list_guidelines.md. As a result, list creators and reviewers are easy to follow and check the requirements: read the repository settings requirements when creating a new list repository; read the list title requirements when typing the list title, ...

You can put the checklist in a separate file, or insert it to the pull request template (so that it appears every time a new pull request is opened), or do the both.

@sindresorhus sindresorhus force-pushed the main branch 3 times, most recently from c05ec08 to bbe1e6f Compare April 9, 2024 14:21
@wongyah wongyah mentioned this pull request Apr 16, 2024
Open
33 tasks
Dwwestcoast

This comment was marked as spam.

Sign in to view

@hlaueriksson hlaueriksson mentioned this pull request Jun 9, 2024
Merged
33 tasks
@sindresorhus sindresorhus force-pushed the main branch 5 times, most recently from f0658c5 to 993cee4 Compare August 8, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dwwestcoast Dwwestcoast Dwwestcoast requested changes

@rsamborski rsamborski rsamborski requested changes

@Cremetruck Cremetruck Cremetruck approved these changes

@Kien329 Kien329 Kien329 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@wongyah @sindresorhus @rsamborski @Dwwestcoast @Kien329 @Cremetruck