Skip to content
This repository was archived by the owner on Nov 28, 2024. It is now read-only.

Feature/using GitHub pull request template#22

Merged
jd merged 1 commit into
Mergifyio:masterfrom
4383:feature/using-github-pull-request-template
Sep 26, 2017
Merged

Feature/using GitHub pull request template#22
jd merged 1 commit into
Mergifyio:masterfrom
4383:feature/using-github-pull-request-template

Conversation

@4383
Copy link
Copy Markdown
Contributor

@4383 4383 commented Sep 25, 2017

Using pull request template file for PR message when is detected in project sources.
Users are invited to fill the template in defined system editor ($EDITOR)

Copy link
Copy Markdown
Member

@jd jd left a comment

Choose a reason for hiding this comment

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

Squash all patches into one commit only.

title = "Pull request for " + end
message = "\n".join(titles)

pr_template = find_pull_request_template()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be done L 147 directly, no need to do the lines above.

Comment thread git_pull_request/__init__.py Outdated
pr_template = find_pull_request_template()
if pr_template:
content = get_pr_template_message(pr_template)
return title, content
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is wrong, this is called when editing a pull request and in this case you don't want to override the content of the existing PR with the template.

Copy link
Copy Markdown
Member

@jd jd left a comment

Choose a reason for hiding this comment

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

Still need to squash those commits :)

Comment thread git_pull_request/tests/__init__.py Outdated


class TestMessageEditor(unittest.TestCase):
class TestMessageEditor(fixtures.TestWithFixtures):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does not seem related?

Comment thread git_pull_request/__init__.py Outdated
return "Pull request for " + end


def generate_message(titles, end):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

both these are only used once so not sure it's worth creating them TBH

Comment thread git_pull_request/__init__.py Outdated
title = generate_title(titles, end)
pr_template = find_pull_request_template()
if pr_template:
LOG.warning(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there's no need for a warning here

Comment thread git_pull_request/__init__.py Outdated
body.write(title + "\n\n")
body.write(message + "\n")
status = os.system(editor + " " + bodyfilename)
status = os.system(get_editor() + " " + bodyfilename)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rather than doing a get_editor() function, do a edit_file function that you can use here and in get_pr_template_message.

@jd
Copy link
Copy Markdown
Member

jd commented Sep 26, 2017

I like the fact that you contribute to git-pull-request without using it it seems ;)

@4383 4383 force-pushed the feature/using-github-pull-request-template branch from 70998e4 to 41ea1ac Compare September 26, 2017 12:39
@4383
Copy link
Copy Markdown
Contributor Author

4383 commented Sep 26, 2017

@jd tester is doubting ;)
squash and refactor done!
Else I've discover this project yesterday and I think it is a good approach to set up a development workflow in a team via this tool

@4383 4383 force-pushed the feature/using-github-pull-request-template branch from 41ea1ac to 2d2fe4a Compare September 26, 2017 13:03
@4383
Copy link
Copy Markdown
Contributor Author

4383 commented Sep 26, 2017

applying your last review done!

Copy link
Copy Markdown
Member

@jd jd left a comment

Choose a reason for hiding this comment

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

a last thing I guess!

Comment thread git_pull_request/__init__.py Outdated
if status != 0:
raise RuntimeError("Editor exited with status code %d" % status)
edit_file(bodyfilename)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can also factorize the read/unlink part IMHO.

just rename to edit_file_get_content_and_remove or something like that if you wish ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done!

@4383 4383 force-pushed the feature/using-github-pull-request-template branch from 2d2fe4a to e899df8 Compare September 26, 2017 13:14
@jd jd merged commit 3fbe4c5 into Mergifyio:master Sep 26, 2017
@4383 4383 deleted the feature/using-github-pull-request-template branch November 17, 2017 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants