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 to ask for unique branch #11200

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

h00die
Copy link
Contributor

@h00die h00die commented Jan 5, 2019

Updates the PR template to ask people to submit from a unique branch.

As per: #11086 (comment) and #11086 (comment)

I'm not set on this as the fix to the overall behavioral problem, but it is a step in the right direction IMO.

@h00die h00die added the easy label Jan 5, 2019
@bcoles
Copy link
Contributor

bcoles commented Jan 6, 2019

I like the idea, given the number of PRs we've had to kill due to merging from master. In some instances, the author was discouraged and never came back.

It might be best to reference a URL, that we control and can update, rather than my template comment reply (which I stole from someone @ r7).

I took a look through the existing msf documentation and couldn't find any relevant references.

The CONTRIBUTING.md states:

Do create a topic branch to work on instead of working directly on master to preserve the history of your pull request. See PR#8000 for an example of losing commit history as soon as you update your own master branch.

topic branch links to this, which is overly verbose and isn't newbie friendly (branches: how do they work?). The page contains a wall of text and multiple diagrams, but no commands to run. Creating a branch for a one-off PR is pretty simple, and shouldn't require multiple diagrams.

The other references for contributors on the wiki aren't helpful either.

Contributing-to-Metasploit states:

Our preferred method of module submission is via a git pull request from a feature branch on your own fork of Metasploit. You can learn how to create one here: https://github.com/rapid7/metasploit-framework/wiki/Landing-Pull-Requests

Following that link takes you to a page which begins by stating in bold This page is meant for Committers. If you are unsure whether you are a committer, you are not. - that's kind of a turn off. The page also makes zero references to "feature branch."

The acceptance guidelines and Setting-Up-a-Metasploit-Development-Environment pages make no reference to feature/topic branches either.

someone-who-isn't-me should probably write up some simple steps somewhere for newbies to create a topic branch. Here's some steps for someone to copypasta somewhere:

# Checkout the master branch
git checkout master

# Create a new branch for your feature
git checkout -b my-cool-new-feature

# Add your new files
git add modules/my-cool-new-module

# Commit your changes with a relevant message
git commit

# Push your changes to GitHub
git push origin my-cool-new-feature

# Now browse to the following URL and create your pull request!
# - https://github.com/rapid7/metasploit-framework/pulls

@busterb
Copy link
Member

busterb commented Jan 9, 2019

@ccondon-r7
Copy link
Contributor

All that is doable. We're working on updated contributor documentation over the next couple of months.

@bcoles
Copy link
Contributor

bcoles commented Jan 9, 2019

Shall we merge this PR then, and circle back later?

@ccondon-r7
Copy link
Contributor

Go for it, @bcoles!

@bcoles bcoles added the rn-no-release-notes no release notes label Jan 10, 2019
@bcoles bcoles self-assigned this Jan 10, 2019
@bcoles bcoles merged commit ed98fc8 into rapid7:master Jan 10, 2019
@bcoles bcoles mentioned this pull request Jan 10, 2019
@h00die h00die deleted the pr_template.md branch January 14, 2019 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants