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 PR Template #4183

Merged
merged 30 commits into from Apr 10, 2019
Merged

Update PR Template #4183

merged 30 commits into from Apr 10, 2019

Conversation

dobooth
Copy link
Contributor

@dobooth dobooth commented Apr 5, 2019

This PR is a:

  • Content update

Summary

When this pull request is merged, it will...
Update the PR template to remove checkboxes and align contribution types to label.
Add a section about selecting a contribution type when submitting a PR.
https://github.com/magento/devdocs/blob/master/.github/CONTRIBUTING.md

@dobooth dobooth self-assigned this Apr 5, 2019
@dobooth dobooth requested a review from shrielenee April 5, 2019 15:20
Copy link
Contributor

@shrielenee shrielenee left a comment

Choose a reason for hiding this comment

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

Approved -- a few minor suggestions. I suggest asking other people on the team to also review this, and post in our Slack channel, so people are aware of this change (and can weigh in if need-be).

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
shrielenee and others added 3 commits April 5, 2019 10:26
Co-Authored-By: dobooth <dobooth@adobe.com>
Co-Authored-By: dobooth <dobooth@adobe.com>
Co-Authored-By: dobooth <dobooth@adobe.com>
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved

Create a PR to the [magento/devdocs repo](https://github.com/magento/devdocs). Fill out as much info as possible and link any GitHub issues.

In general, you should use `master` as the base branch when creating a PR. If your contribution is related to a release that is in progress, use a version-specific integration branch, like `2.3.1-integration`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

develop is a release integration branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like we should change this when we officially make the repo changes.

Copy link
Collaborator

@dshevtsov dshevtsov Apr 5, 2019

Choose a reason for hiding this comment

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

We've been using the develop branch officially:
https://github.com/magento/devdocs/tree/develop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@dshevtsov dshevtsov left a comment

Choose a reason for hiding this comment

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

There is an ambiguity that makes use of these types confusing along with labels.
Too many entities for a PR type specification.
It would make sense to use here the same options that we use for labels:

  • New topic
  • Major update
  • Technical
  • Editorial
  • Site improvements
  • Bug

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
- [ ] Content update
- [ ] Content fix or rewrite
- [ ] Bug fix or improvement
Describe the goal and the types of changes this PR covers.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Describe the goal and the types of changes this PR covers.
Describe the goal and the type of changes this PR covers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a list of types.

It should correspond to the labels we use. The labels already contain short descriptions, so you could just copy the label description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea.

## Additional information

List all affected URLs
List all files that this PR changes:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The list of files is always available in the GitHub UI at Files changed tab

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is nice to have up front for editors, groomers...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Short list of the files is also available at Jump to dropdown menu that can be used by editors and groomers.
This list can be very big, and can change during a PR history. Duplicating and maintaining it manually can be confusing for contributors.
Screen Shot 2019-04-05 at 14 18 48


<!--
Thank you for your contribution!
If this PR references a file in a Magento repository, add it here.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
If this PR references a file in a Magento repository, add it here.
If this PR references a file in a Magento codebase repository, add it here.

<!-- (REQUIRED) The Url that this PR will modify -->

<!-- (OPTIONAL) What other information can you provide about this PR? -->
## Source code URLS (remove if unused)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## Source code URLS (remove if unused)
## Links to source code (remove if unused)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.github/CONTRIBUTING.md Show resolved Hide resolved
.github/CONTRIBUTING.md Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
meker12 and others added 5 commits April 5, 2019 14:09
Co-Authored-By: dobooth <dobooth@adobe.com>
Co-Authored-By: dobooth <dobooth@adobe.com>
Co-Authored-By: dobooth <dobooth@adobe.com>
Co-Authored-By: dobooth <dobooth@adobe.com>
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved

## Additional information
## Affected URLS (required)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to URLs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@dobooth dobooth changed the title Added info about setting contribution types Update PR Template Apr 8, 2019
Copy link

@lorikrell lorikrell left a comment

Choose a reason for hiding this comment

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

Tossed in some comments.

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
- [ ] Content fix or rewrite
- [ ] Bug fix or improvement
We set labels (and therefore points) based on the scope of changes with a PR.
Note the scope of this PR by leaving one of the options below and deleting the others.

Choose a reason for hiding this comment

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

Maybe move this line before the labels one to call for action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@osrecio
Copy link
Member

osrecio commented Apr 9, 2019

Impressed with the feedback!

Good job! :)


Feel free to remove this section before creating this PR.
`master` is the default branch. Be sure to change the base if your PR is for another branch.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like an explanation of what master means to us, and when to use develop or some other branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@dobooth
Copy link
Contributor Author

dobooth commented Apr 10, 2019

running tests

@dobooth dobooth merged commit f06a574 into master Apr 10, 2019
@ghost
Copy link

ghost commented Apr 10, 2019

Hi @dobooth, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@dobooth dobooth deleted the db_contributing branch April 10, 2019 15:54
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.

None yet

10 participants