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

Update pull_request_template.md #322

Merged
merged 10 commits into from
Feb 25, 2020
Merged

Update pull_request_template.md #322

merged 10 commits into from
Feb 25, 2020

Conversation

jessicaschilling
Copy link
Contributor

First pass at an improved PR template for awesome-ipfs. @andrew -- we'll need to change the pre-submit checklist based on what you figure out surrounding the mechanisms of making a PR easier to make. In the meantime, please take a look at the remaining questions to see if they're useful for getting a bead on collabs.

cc @momack2 @autonome @renrutnnej

First pass at an improved PR template for awesome-ipfs. @andrew -- we'll need to change the pre-submit checklist based on what you figure out surrounding the mechanisms of making a PR easier to make. In the meantime, please take a look at the remaining questions to see if they're useful for getting a bead on collabs.

cc @momack2 @autonome @renrutnnej
@jennwrites
Copy link

I wonder if we should include a link to the confirm review of the project code of conduct as well as the content policy? Thoughts?

@RubenKelevra
Copy link
Contributor

The first section should be no "checklist", but just a point list.

A checklist entry should be to confirm that the changes only cover "one" item of the list above, and ask the user to write which one... or similar.

The advantage of a checklist is that you can see the "progress" of a PR in terms of checklist completion in the PR overview as bar.

When we expect the user not to check everything, this bar is not useful.

@jessicaschilling
Copy link
Contributor Author

Hi @RubenKelevra -- you raise a good point about checklists being used for off-label purposes. It isn't ideal to use GitHub checklists for "check one" purposes, because of the impact on a progress bar, but I'm suggesting that the alternatives would have a more negative impact on user experience:

  • Using a mechanism like "delete all the things in this list that don't apply" is more difficult for the user than just checking one or several boxes, and increases the chance that the user just won't bother
  • Forcing the user off-site to an external mechanism designed for this sort of question-and-answer (a Google survey, or whatnot) disrupts entirely the experience of making the PR, and increases the chance that the user won't bother, or will abandon the PR entirely

For this reason, proposing that we keep using the "check one" boxes, as we have previously in this PR template, but revisit later if we find it's a blocker.

@RubenKelevra
Copy link
Contributor

True, user experience is key here. Good idea to postpone this after the PR has been completed.

@RubenKelevra
Copy link
Contributor

This is somewhat related and might block this PR until decided. I really like to clean up the categories a bit to make it easier for the users to understand the content (e.g. the difference between a commercial service and an open source project you can host yourself).

Maybe we can discuss first the cleanup of the categories, to directly incorporate the good questions for the maintainers to select the proper category.

I volunteer to review and resort the content, and create a PR for RFC afterwards.

#323 (comment)

@jessicaschilling
Copy link
Contributor Author

@RubenKelevra -- thanks for your work on #323. I think we can approach this iteratively, and update the PR template again later if/as needed when the categorization logic changes.

@andrew, do you have write access to this repo? I think we could merge this version now, and amend it again later once you've taken a closer look at the merge process and how we might be able to make that easier. Thoughts? If you're OK with that, do you mind merging?

Copy link
Contributor

@RubenKelevra RubenKelevra 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 to me

Copy link
Contributor

@andrew andrew left a comment

Choose a reason for hiding this comment

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

Suggested to change formatting of the hints to html comments so they disappear from the PR body once it's been submitted

pull_request_template.md Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
pull_request_template.md Outdated Show resolved Hide resolved
@jessicaschilling
Copy link
Contributor Author

@andrew - good idea on the comment lines. I've made the changes -- can you please give another look and merge if you're cool with it?

Copy link
Contributor

@andrew andrew left a comment

Choose a reason for hiding this comment

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

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.

4 participants