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.

Sign up for GitHub

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

PR Template Updates #22

Merged
merged 3 commits into from
Apr 4, 2024
Merged

PR Template Updates #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f140f3
Adding D&A Dependencies reminder to PR Template
danielwheeler1987 Apr 3, 2024
4d87c42
Making Tailor Made & Premium Blend owners of PR Template
danielwheeler1987 Apr 3, 2024
9f27b78
Use warning emoji for risk levels in PR template
chmurph2 Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
* @customink/atlas

.github/pull_request_template.md @customink/tailor-made @customink/premium-blend
15 changes: 10 additions & 5 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ Provide a concise summary of the motivation and the driving force behind this ch
_[Replace this with a description of the risk, if any, and how the change will be deployed.]_

<!-- remove any that do not apply -->
- ✅ Big/complex change
- ✅ Big splash zone
- ✅ High stakes if errors occur
- ✅ Low confidence
- ✅ Not hidden by feature flag
- ⚠️ Big/complex change
- ⚠️ Big splash zone
- ⚠️ High stakes if errors occur
- ⚠️ Low confidence
- ⚠️ Not hidden by feature flag
- ✅ Negligible risk!

### Changes
Expand Down Expand Up @@ -69,6 +69,11 @@ Common, optional tasks are included here in case you forgot something important.
- Are there any new queries in your change set that might require new indexes?
- Do any new queries require time-boxing to avoid table-scans when the data grows?


##### Data & Analytics Dependencies
- For customer-facing applications, are there any GA tagging changes that are needed or created by this change? If so, have those been discussed with D&A?
- Has Data Engineering been notified of any schema changes?
Copy link
Member

Choose a reason for hiding this comment

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

This should mainly be for modifications of existing schema attributes. Additions shouldn't matter. I don't know the details of what was discussed previously around this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Has Data Engineering been notified of any schema changes?" Should cover all the basis here.

Copy link
Contributor

@Shimmi Shimmi Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Has Data Engineering been notified of any schema changes

Not sure this is the best wording or requirement. We do not do that unless it results in an impact on Datafuse configuration. Even if it does, we tend to prepare a PR to change that and run that true Data team.


### What GIF Best Describes This Pull Request?

<!--
Expand Down