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

ISSUE_TEMPLATE, PULL_REQUEST_TEMPLATE guidelines updated. #10

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

Conversation

optimistanoop
Copy link

Fixes # NA

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream master branch.
  • I have added/updated necessary tests/documentation (if applicable)

Changes proposed in this pull request:

Code of conduct , issue template and pull request template guidelines are among 6 key features for community profile for our project.
Ref:- https://github.com/UdacityMobileWebScholarship/showcase-app/community

This PR will help our project stand out in terms of recommended community standards.
Templates provided in the PR will also help our contributors while creating PR and issues with standard guidelines by showing them the template as the last time checklist template in the description text area provided.

@@ -31,6 +31,19 @@ a proposal for your work first, to be sure that we can use it.
Before you submit an issue, please search the issue tracker, maybe an issue for your identified problem already exists and you can join the discussion there only.
For bugs, please try to provide as much information possible to reproduce it as possible, that will really help us a lot!

## Report bugs using Github's [issues](https://github.com/UdacityMobileWebScholarship/showcase-app/issues)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@optimistanoop There is no need of this as it is already mentioned how to report a bug. Please remove this.

**I'm submitting a ...**
- [ ] bug report
- [ ] feature request
# Template to submit an issue
Copy link
Collaborator

Choose a reason for hiding this comment

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

@optimistanoop Only add

- TODO - After reading this template you can remove this template while submitting an issue -->


#### I'm submitting this issue because:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@optimistanoop Please keep it as it was before. No need to add extra checkboxes as added things can be included in Bug Report.

@@ -9,5 +12,8 @@ Fixes #

#### Changes proposed in this pull request:

-
-
* Describe how this PR will make our project even more awesome.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@optimistanoop No need to add this as Heading Changes proposed in this pull request conveys the same message.

@@ -9,3 +25,21 @@
<!-- Behavior would be without the bug. -->

**Steps to reproduce:**
<!-- Give a complete description of how to reproduce the problem. -->

### supporting information:
Copy link
Collaborator

Choose a reason for hiding this comment

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

It can be included in Current behavior. No need to add this.

- [ ] `node -v` prints:
- [ ] Windows, OS X/macOS, or Linux?:

<!-- For feature requests, uncomment the section below. But first, review the existing feature requests and make sure there isn't one that already describes the feature you'd like to see added: -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

@optimistanoop No need to add these as well before giving feature request, the contributor will discuss all these things on the issue and there is no need to rewrite it again here.

@ParthS007
Copy link
Collaborator

@optimistanoop Please open an issue before sending a PR.

@ParthS007 ParthS007 added the PR: change requested Some change is required before merging this PR. label Apr 19, 2018
@optimistanoop
Copy link
Author

@ParthS007 Required changes are done now. Kindly have a look.

Copy link
Collaborator

@ritikrishu ritikrishu left a comment

Choose a reason for hiding this comment

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

see comment inline. wait for someone from collaborators team to respond before adding PR close warning.

-
-
<!-- Describe how this PR will make our project even more awesome.
If this solves an issue, please reference it in this PR. -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

the Fixes # thing at the top is to reference Issues, so I don't think it's necessary here as well.
I see people are making few PRs without opening an issue, should we mention in template that your PR will be closed without merge if we do not see issue tagged with it. @divyamrastogi ?

Copy link
Collaborator

@ritikrishu ritikrishu Apr 20, 2018

Choose a reason for hiding this comment

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

If this solves an issue

is not the case, because a PR should close at least 1 issue from the issue board.

Copy link
Author

@optimistanoop optimistanoop Apr 23, 2018

Choose a reason for hiding this comment

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

@ritikrishu @ParthS007 Please tell me the steps required (changes) from my side to close this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hey @optimistanoop, sorry I missed your comment here somehow. So the comment in PR template that says "If this solves an issue, please reference it in this PR", I'd like to mention, "Please provide reference to active issue here" (at the top where it says Fixes #my_issue_number). Also a warning, "An active issue is required for a PR to be reviewed. If you do not provide one, you pull request might be closed by collaborators"

Copy link
Author

Choose a reason for hiding this comment

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

@ritikrishu @ParthS007 Review changes are done. Please have a look and close this.

@palnabarun
Copy link
Collaborator

@optimistanoop Are you still working on it?

@optimistanoop
Copy link
Author

@palnabarun Review changes are done. Please have a look and close this. Thanks

@palnabarun palnabarun self-requested a review May 15, 2018 07:14
@@ -1,11 +1,23 @@
**I'm submitting a ...**
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the earlier text was better.

Copy link
Author

Choose a reason for hiding this comment

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

@palnabarun @ritikrishu @ParthS007 I feel a lot of friction in the process of review/ approval.
I think you all moderators should agree to a same point. Its impossible implement multiple thoughts on the same change.

Secondly, In this PR, I have made changes more than 3-4 times based on all reviews given by all moderators. So for the sake of simplicity and fast review/approval system, please suggest a final change and lets close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: change requested Some change is required before merging this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants