Skip to content

Add PR template #991

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.

Sign up for GitHub

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

Merged
ImperatorS79 merged 34 commits into from
Jun 16, 2019
Merged

Add PR template #991

Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
06ad52f
Merge pull request #1 from PhoenicisOrg/master
ImperatorS79 Aug 28, 2017
86fd07c
Update script.js
ImperatorS79 Aug 28, 2017
a6f232b
Merge pull request #2 from PhoenicisOrg/master
ImperatorS79 Aug 28, 2017
a80aa84
Merge pull request #3 from PhoenicisOrg/master
ImperatorS79 Sep 26, 2017
6da4a59
Merge pull request #4 from PhoenicisOrg/master
ImperatorS79 Mar 19, 2018
4a11f65
Merge pull request #8 from PhoenicisOrg/master
ImperatorS79 Apr 9, 2018
b2d5724
Merge pull request #9 from PhoenicisOrg/master
ImperatorS79 May 7, 2018
b7d1f0f
Merge pull request #10 from PhoenicisOrg/master
ImperatorS79 May 13, 2018
533df6e
Merge pull request #11 from PhoenicisOrg/master
ImperatorS79 May 22, 2018
186e4ed
Merge pull request #12 from PhoenicisOrg/master
ImperatorS79 May 29, 2018
99cf1db
Merge pull request #13 from PhoenicisOrg/master
ImperatorS79 Jun 13, 2018
87999b4
Merge pull request #15 from PhoenicisOrg/master
ImperatorS79 Oct 10, 2018
610b258
Merge pull request #16 from PhoenicisOrg/master
ImperatorS79 Oct 12, 2018
ad2ea01
Merge pull request #17 from PhoenicisOrg/master
ImperatorS79 Jan 12, 2019
2e53a22
Merge pull request #18 from PhoenicisOrg/master
ImperatorS79 Jan 18, 2019
10f93ca
Merge pull request #20 from PhoenicisOrg/master
ImperatorS79 Feb 2, 2019
ade899d
Merge pull request #21 from PhoenicisOrg/master
ImperatorS79 Feb 27, 2019
db6e1b9
Merge pull request #23 from PhoenicisOrg/master
ImperatorS79 May 9, 2019
da1a517
Merge pull request #26 from PhoenicisOrg/master
ImperatorS79 May 20, 2019
72863e6
Merge pull request #28 from PhoenicisOrg/master
ImperatorS79 Jun 2, 2019
85c531c
Create pull_request_template.md
ImperatorS79 Jun 7, 2019
12129a7
Update pull_request_template.md
ImperatorS79 Jun 7, 2019
65288f1
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
982a7be
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
21de932
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
a97a355
Where is my spelling
ImperatorS79 Jun 8, 2019
a96c726
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
e89adde
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
503ed7e
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
59e39ed
Update pull_request_template.md
ImperatorS79 Jun 8, 2019
2723749
Update pull_request_template.md
ImperatorS79 Jun 9, 2019
cd4e732
Update pull_request_template.md
ImperatorS79 Jun 9, 2019
3207141
Update pull_request_template.md
ImperatorS79 Jun 14, 2019
7ec5969
Update pull_request_template.md
ImperatorS79 Jun 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Where is my spelling
  • Loading branch information
@ImperatorS79
ImperatorS79 authored Jun 8, 2019
commit a97a35577eec6e349bea3b53830deac48b7a71b8
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ Hardware (GPU/CPU):
[ ] Script tested as a regular phoenicis user and working (if you have a problem -> draft and ask for help)
[ ] json-align and eslint run accorded to the .
Copy link
Copy Markdown
Collaborator

@plata plata Jun 8, 2019

Choose a reason for hiding this comment

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

according to the README?

Copy link
Copy Markdown
Collaborator

@plata plata Jun 8, 2019

Choose a reason for hiding this comment

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

Should be a link.


Codacy and travis must run perfectly for the Pr to be reviewed. You can however ping them if you think there is a problem with those tools (use draft for this).
Codacy and travis must run perfectly for the PR to be reviewed. You can ping reviewers if you think there is a problem with those tools (use draft for this).
Copy link
Copy Markdown
Collaborator

@plata plata Jun 8, 2019

Choose a reason for hiding this comment

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

The last sentence is not clear for me.

Copy link
Copy Markdown
Contributor Author

@ImperatorS79 ImperatorS79 Jun 8, 2019

Choose a reason for hiding this comment

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

What happens if travis fails without any reason for example.

Copy link
Copy Markdown
Collaborator

@plata plata Jun 8, 2019

Choose a reason for hiding this comment

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

I see. Nevertheless, I'm not sure the approach is useful because as far as I know Travis and Codacy will not run for draft pull requests. So you would have to set the PR to draft after one of the checks fail. That doesn't make too much sense in my opinion. Apart from that, everybody would have to remove the remark from the description because it doesn't really belong to the description. Therefore I would rather go for the checklist only.

Copy link
Copy Markdown
Contributor Author

@ImperatorS79 ImperatorS79 Jun 8, 2019

Choose a reason for hiding this comment

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

I would like to at least keep an info about codacy and travis to be checked because it is really annoying to always remind to someone "Please check travis and codacy".

Copy link
Copy Markdown
Collaborator

@plata plata Jun 9, 2019

Choose a reason for hiding this comment

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

Ok but this could also be a point of the checklist, right?