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

chore: expand pull request template with action items #2509

Merged

Conversation

pragmaticivan
Copy link
Contributor

Which problem is this PR solving?

  • From Sep 29th, 2021 meeting. This Pr introduces some Action Items to remind contributors and help reviewers with broader changes that might include documentation updates and perhaps how to replicate something.

@pragmaticivan pragmaticivan requested a review from a team September 29, 2021 17:26
@pragmaticivan
Copy link
Contributor Author

It might make sense to incorporate the hidden/commented messages in this PR somehow?

@codecov
Copy link

codecov bot commented Sep 29, 2021

Codecov Report

Merging #2509 (9b1ed94) into main (f2a5624) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2509      +/-   ##
==========================================
- Coverage   93.06%   93.05%   -0.01%     
==========================================
  Files         132      139       +7     
  Lines        4856     5172     +316     
  Branches     1040     1110      +70     
==========================================
+ Hits         4519     4813     +294     
- Misses        337      359      +22     
Impacted Files Coverage Δ
packages/opentelemetry-sdk-trace-web/src/utils.ts 94.90% <0.00%> (ø)
...entelemetry-sdk-trace-web/src/WebTracerProvider.ts 100.00% <0.00%> (ø)
...telemetry-sdk-trace-web/src/StackContextManager.ts 94.28% <0.00%> (ø)
...-sdk-trace-web/src/enums/PerformanceTimingNames.ts 100.00% <0.00%> (ø)
...ry-context-zone-peer-dep/src/ZoneContextManager.ts 85.18% <0.00%> (ø)
...es/opentelemetry-context-zone-peer-dep/src/util.ts 100.00% <0.00%> (ø)
packages/opentelemetry-sdk-trace-web/src/types.ts 100.00% <0.00%> (ø)

@dyladan
Copy link
Member

dyladan commented Sep 29, 2021

The commented lines only show up to the person who is authoring the PR. They are hidden if the PR author leaves it it when they click submit.

Copy link
Member

@rauno56 rauno56 left a comment

Choose a reason for hiding this comment

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

Yep, if any, those instructions could be hidden comments, because they don't add much for the reader.

What I've personally found, is that the checklists are useful in only occasionally(which IMHO talks for them - such a simple means for some benefit). I hope people just take those as reminders and nothing to religiously follow. If we really have a strong rule, we should be able to automate enforcing it as "following guidelines" is enforced by the linter.

.github/PULL_REQUEST_TEMPLATE.md Show resolved Hide resolved
Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>
@vmarchaud vmarchaud merged commit c160ad8 into open-telemetry:main Oct 17, 2021
@pragmaticivan pragmaticivan deleted the chore/expand-pull-request-template branch October 17, 2021 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants