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

Projects
None yet
5 participants
@h00die
Copy link
Contributor

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

This comment has been minimized.

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
@bcoles

bcoles approved these changes Jan 6, 2019

@busterb

This comment has been minimized.

Copy link
Contributor

busterb commented Jan 9, 2019

@ccondon-r7

This comment has been minimized.

Copy link
Contributor

ccondon-r7 commented Jan 9, 2019

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

@bcoles

This comment has been minimized.

Copy link
Contributor

bcoles commented Jan 9, 2019

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

@ccondon-r7

This comment has been minimized.

Copy link
Contributor

ccondon-r7 commented Jan 9, 2019

Go for it, @bcoles!

@bcoles bcoles self-assigned this Jan 10, 2019

@bcoles bcoles merged commit ed98fc8 into rapid7:master Jan 10, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

bcoles added a commit that referenced this pull request Jan 10, 2019

msjenkins-r7 added a commit that referenced this pull request Jan 10, 2019

@bcoles bcoles referenced this pull request Jan 10, 2019

Open

how2uniquebranch #11224

@h00die h00die deleted the h00die:pr_template.md branch Jan 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment