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

Add issue and pull request templates #241

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Add issue and pull request templates #241

merged 6 commits into from
Mar 30, 2017

Conversation

Liquidsoul
Copy link
Collaborator

@Liquidsoul Liquidsoul commented Mar 29, 2017

Checklist

  • I've checked that all new and existing tests pass.
  • I've updated the documentation if necessary.

Description

This adds issue and pull requests templates to help users and contributors.
I used the fastlane's ones as a reference.
Any feedback and suggestions welcome!

Motivation and Context

This is an implementation for #210

- [ ] I have tried with the latest version of OHHTTPStubs
- [ ] I have read the [README](https://github.com/AliSoftware/OHHTTPStubs/blob/master/README.md)
- [ ] I have read the [Using the right Swift Version of `OHHTTPStubs` for your project](https://github.com/AliSoftware/OHHTTPStubs#using-the-right-swift-version-of-ohhttpstubs-for-your-project) section
- [ ] I have searched in the [existing issues](https://github.com/AliSoftware/OHHTTPStubs/issues?utf8=✓&q=is%3Aissue)
Copy link
Owner

@AliSoftware AliSoftware Mar 29, 2017

Choose a reason for hiding this comment

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

- [ ] I have read [the OHHTTPStubs wiki](https://github.com/AliSoftware/OHHTTPStubs/wiki) to see if there wasn't a detailed page talking about my issue


##### Complete output when you encounter the issue (if any)

[INSERT OUTPUT HERE]
Copy link
Owner

Choose a reason for hiding this comment

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

We should insert 3-backticks before and after that line to encourage people to put their output in them and have a nicely-formatted terminal output

We could even prepare a <details> block for them (so that if the terminal output they paste is long it doesn't make the issue description too unreadable), but I'm afraid doing that would be disturbing to people not knowing this tag

@@ -0,0 +1,24 @@
### New Issue Checklist
Copy link
Owner

Choose a reason for hiding this comment

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

There's an HTML comment on top of the PULL_REQUEST_TEMPLATE.md down below, why not adding a similar one here too?

<!-- Thanks for taking the time to report your issue with _OHHTTPStubs_! When submitting your issue, please make sure to check the following boxes by putting an x in the appropriate [ ] so we can fully understand the context of your problem and help you better -->


### Checklist
- [ ] I've checked that all new and existing tests pass.
- [ ] I've updated the documentation if necessary.
Copy link
Owner

Choose a reason for hiding this comment

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

- [ ] I've added an entry in the CHANGELOG to credit myself

@@ -0,0 +1,13 @@
<!-- Thanks for contributing to _OHHTTPStubs_! Before you submit your pull request, please make sure to check the following boxes by putting an x in the [ ] -->

### Checklist
Copy link
Owner

Choose a reason for hiding this comment

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

Please add an empty newline after markdown section titles (it's my OCD talking 😄)

- [ ] I've checked that all new and existing tests pass.
- [ ] I've updated the documentation if necessary.

### Description
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto, empty line

### Description
<!--- Describe your changes in detail -->

### Motivation and Context
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto, empty line

@@ -21,4 +24,4 @@

##### Complete output when you encounter the issue (if any)

[INSERT OUTPUT HERE]
```[INSERT OUTPUT HERE]```
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry I wasn't clear, given that 3-backticks blocks are supposed to contain multiple lines of code, I expected to have them in dedicated lines to let them wrap a block (and not be inline), so more like this:

```
[INSERT OUTPUT HERE]
```

@@ -1,13 +1,17 @@
<!-- Thanks for contributing to _OHHTTPStubs_! Before you submit your pull request, please make sure to check the following boxes by putting an x in the [ ] -->
Copy link
Owner

Choose a reason for hiding this comment

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

I just realised that HTML comments like those, not being markdown, won't be rendered, so that means that using underscores here isn't really useful. Maybe remove the underscores around OHHTTPStubs to make it read better when unformatted? (same for the issue template obviously)

@@ -1,9 +1,12 @@
<!-- Thanks for taking the time to report your issue with _OHHTTPStubs_! When submitting your issue, please make sure to check the following boxes by putting an x in the appropriate [ ] so we can fully understand the context of your problem and help you better -->
Copy link
Owner

Choose a reason for hiding this comment

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

Now that I'm reading that again, the part "putting an x in the appropriate [ ]" feels like it's singular ("in the appropriate box") while we actually want to mean "in the appropriate boxes" — as the contributor should not limit their choices to only one box. Maybe changing the formulation to "putting an x in each appropriate [ ]" would help make that clearer?

### Checklist

- [ ] I've checked that all new and existing tests pass.
- [ ] I've updated the documentation if necessary.
Copy link
Owner

Choose a reason for hiding this comment

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

None of the other checklist items end with a period (.), so why those two? 🤔
(I'm OK for either style… as long as it's applied consistently 😄 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll remove the dots there then 👍


##### Complete output when you encounter the issue (if any)

```[INSERT OUTPUT HERE]```
Copy link
Owner

Choose a reason for hiding this comment

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

Don't forget to address this one too 😉

```
[INSERT OUTPUT HERE]
```

@AliSoftware
Copy link
Owner

Perfect ! Ready to merge once Travis wakes up.

@AliSoftware AliSoftware merged commit 8ecb062 into master Mar 30, 2017
@AliSoftware AliSoftware deleted the templates branch November 5, 2017 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants