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: pr template #359

Merged
merged 2 commits into from
May 29, 2024
Merged

Add: pr template #359

merged 2 commits into from
May 29, 2024

Conversation

teesloane
Copy link
Contributor

I don't have to write anything here because there is no template yet.

😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉 😉


- [ ] I have performed a self-review of my code
- [ ] If it is a core feature, I have added tests.
- [ ] Do we need to implement analytics?
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I donno, it came from a template 🤷

Mostly, I think of it as—is there tracking that needs to be implemented alongside the business logic?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's maybe overkill - otherwise ship it?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's a bit open to interpretation so maybe not super helpful

Copy link
Member

@sereprz sereprz left a comment

Choose a reason for hiding this comment

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

LGTM


- [ ] I have performed a self-review of my code
- [ ] If it is a core feature, I have added tests.
- [ ] Do we need to implement analytics?
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a bit open to interpretation so maybe not super helpful

@teesloane teesloane merged commit ef1df94 into main May 29, 2024
1 check passed
@teesloane teesloane deleted the ty/pr-template branch May 29, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants