WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

[1] PR & issue template fixes #1 #2

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

Open
w1stler wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[1] PR & issue template fixes #1 #2

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97f69b5
[1] PR & issue template
Dec 4, 2019
f37543b
[1] PEBKAC
w1stler Dec 5, 2019
e55d2dd
[1] Remove rollback from PR template
w1stler Jan 8, 2020
dd5ac33
[1] Remove unit tests from template
w1stler Mar 19, 2020
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
16 changes: 16 additions & 0 deletions ISSUE_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#### Subject of the issue:
Describe your issue here

#### Your environment:
Copy link

@OtisRed OtisRed Dec 5, 2019

Choose a reason for hiding this comment

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

Do we need this, while working in docker?

Copy link
Member Author

@w1stler w1stler Dec 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

By environment I meant production / RC / local. I am not sure if deployed envs' configuration will differ.

* version/branch
* which browser and its version
Copy link
Member

@arturtamborski arturtamborski Dec 5, 2019

Choose a reason for hiding this comment

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

I don't think that there's much of use out of it, we can probably assume that every member is using a "recent enough" one.

Copy link
Member Author

@w1stler w1stler Dec 11, 2019

Choose a reason for hiding this comment

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

Ditto, I would leave it as a good practice.


#### Steps to reproduce:
Tell us how to reproduce this issue

#### Actual behaviour:
Tell us what happened

#### Expected behaviour:
Tell us what should happen

20 changes: 20 additions & 0 deletions PULL_REQUEST_TEMPLATE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#### Story / Bug id:
123 [link to the issue]

#### Description:
Describe changes made in your pull request here

#### Migrations:
N/A

#### Rollback:
Copy link

@OtisRed OtisRed Dec 5, 2019

Choose a reason for hiding this comment

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

Isn't it too specific? As I understand rollback means reversed migration (does it?), in which case we would hardly ever use that field and even if we do, "Migrations" still seems to be the likely place I would mention it

Copy link
Member Author

@w1stler w1stler Dec 11, 2019

Choose a reason for hiding this comment

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

Defaultly it would be N/A (or the previous version mentioned), but I would stick with that as a good practice. Nevertheless could be removed. ;-)

N/A

#### New imports / dependencies:
N/A

#### Unit Tests:
N/A
Copy link
Member

@arturtamborski arturtamborski Dec 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

What does it mean? Should I list unit tests here? Or describe what I (potentially) added to the testing suite?

Copy link

@OtisRed OtisRed Dec 5, 2019

Choose a reason for hiding this comment

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

When you don't have a habit of writing tests this could be actually self-reflective. "Oh so... I wrote this function so what kind of tests do I need here? Since it is expected to list sth...".

Also this could be early checker for complexity od the code. If you have a lot of those the chance is that you like spaghetti, aren't you? :D

Copy link
Member Author

@w1stler w1stler Dec 11, 2019

Choose a reason for hiding this comment

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

Hard-copy from other project. :D Could be removed.


#### What tests do I need to run to validate this change:
N/A
Copy link
Member

@arturtamborski arturtamborski Dec 5, 2019

Choose a reason for hiding this comment

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

👍 💯