-
Notifications
You must be signed in to change notification settings - Fork 406
chore: remove plugin checklist from PR template #7105
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
Changes from 7 commits
248575b
73389dc
a8f7cc0
f57420b
4bc6b4a
914868d
2dac666
b4e44e0
dd687c7
5da63dc
5d21757
45f61a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,8 @@ | ||
| <ins>**Please make sure your changes are properly tested**!</ins> | ||
|
|
||
| ### What does this PR do? | ||
| <!-- A brief description of the change being made with this pull request. --> | ||
|
|
||
| ### Motivation | ||
| <!-- What inspired you to submit this pull request? --> | ||
|
|
||
| ### Plugin Checklist | ||
| <!-- Fill this section if adding or updating a plugin. Remove otherwise. --> | ||
|
|
||
| - [ ] Unit tests. | ||
| - [ ] Integration tests. | ||
| - [ ] Benchmarks. | ||
| - [ ] TypeScript [definitions][1]. | ||
| - [ ] TypeScript [tests][2]. | ||
| - [ ] API [documentation][3]. | ||
| - [ ] CI [jobs/workflows][4]. | ||
|
|
||
| [1]: https://github.com/DataDog/dd-trace-js/blob/master/index.d.ts | ||
| [2]: https://github.com/DataDog/dd-trace-js/blob/master/docs/test.ts | ||
| [3]: https://github.com/DataDog/documentation/blob/master/content/en/tracing/trace_collection/library_config/nodejs.md | ||
| [4]: https://github.com/DataDog/dd-trace-js/blob/master/.github/workflows/plugins.yml | ||
|
|
||
| ### Additional Notes | ||
| <!-- Anything else we should know when reviewing? --> | ||
|
simon-id marked this conversation as resolved.
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -464,4 +464,20 @@ most efficient algorithm. To run your benchmark, use: | |
| $ yarn bench | ||
| ``` | ||
|
|
||
| ### PR Checklist | ||
|
|
||
| Here is a simple checklist to help write a more complete PR: | ||
| - [ ] Unit tests. | ||
| - [ ] Integration tests. | ||
| - [ ] Benchmarks. | ||
| - [ ] TypeScript [definitions][2]. | ||
| - [ ] TypeScript [tests][3]. | ||
| - [ ] API [documentation][4]. | ||
| - [ ] CI [jobs/workflows][5]. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so people that want to use the checklist can copy paste it in their description or somewhere else ;)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's meant to be copied it should be a code block, otherwise it's not possible. In its current state, it just shows disabled checkboxes that can't be checked
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I question the usefulness of this and would probably vote to just remove this section altogether
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with removing it. It was a request from @bengl on my original slack thread.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think he intended for people to click "edit source" to actually use it 😉
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'll have to let him say what he intended himself, don't wanna put words into his mouth. Although, once again it's not clicking "edit source", it's clicking the file then the tab "code", which is available for all markdown files to see the original code instead of the markdown render.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intent of this is not to show it nicely, but to make it easy to copy into your PR. So if we want to keep it (which I still doubt the usefulness of), it should be turned into a code block. If you show it "nicely", many users will just be very confused about why they can't click the check boxes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it completely 👍 not sure it's useful either |
||
|
|
||
|
|
||
| [1]: https://docs.datadoghq.com/help | ||
| [2]: https://github.com/DataDog/dd-trace-js/blob/master/index.d.ts | ||
| [3]: https://github.com/DataDog/dd-trace-js/blob/master/docs/test.ts | ||
| [4]: https://github.com/DataDog/documentation/blob/master/content/en/tracing/trace_collection/library_config/nodejs.md | ||
| [5]: https://github.com/DataDog/dd-trace-js/blob/master/.github/workflows/plugins.yml | ||

Uh oh!
There was an error while loading. Please reload this page.