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

Al/new data pr template #13

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Al/new data pr template #13

wants to merge 3 commits into from

Conversation

ALidghi
Copy link

@ALidghi ALidghi commented Jul 28, 2023

Fixes

Fixes #12 by @ALidghi

Description

This PR creates a new PR template for the data team

Technical details

Definition of Done

  • I followed the Arkhn Code Book (I swear!).
  • I have written tests for the code I added or updated.
  • I have updated the documentation according to my changes.
  • I have updated the deployment configuration if needed.

Tests

Copy link

@Jasopaum Jasopaum left a comment

Choose a reason for hiding this comment

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

I might be wrong but I don't think github allows multiple PR templates as it does for issues

@nrogerarkhn
Copy link

I might be wrong but I don't think github allows multiple PR templates as it does for issues

It is possible (see this documentation).

However, usage might be a bit overwhelming if the UI doesn't allow to choose explicitly:
image

image

- [ ] I have checked if the model is SQLFLUFF compliant
- [ ] I have validated the model (dbt run works on my branch)
- [ ] I have run FHIR validate if needed

Choose a reason for hiding this comment

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

To add as below?

  • I have shared this issue with my team (channel data-team)

Copy link
Author

Choose a reason for hiding this comment

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

The first part of the template is about creating a new model and not about an issue, that's why we kept this box only in the issue section of the template :)

@ALidghi
Copy link
Author

ALidghi commented Aug 1, 2023

I might be wrong but I don't think github allows multiple PR templates as it does for issues

It is possible (see this documentation).

However, usage might be a bit overwhelming if the UI doesn't allow to choose explicitly: image

image

Indeed, there is the issue of dealing with multiple templates to address. It seems that this stackoverflow thread adresses it by suggesting two non-perfect solutions :

I found a more convenient solution for this today. The problem with the current answer(s) is that you either need to create a custom URL, or create an additional template outside of the PULL_REQUEST_TEMPLATE directory.

The alternative is to use a symlink, like this:

cd .github
ln -s PULL_REQUEST_TEMPLATE/pull_request_template.md pull_request_template.md
git add pull_request_template.md
git commit -m "Add symlink to default pull request template"
Once merged into the default branch, any new pull requests created through github.com will use the linked template.

And

You can create a manual template selection, such that at least developers who open a PR can click on a link to get to their respective template:

Assuming you have two templates group_a_template.md and group_b_template.md under .github/PULL_REQUEST_TEMPLATE

Create the default template .github/pull_request_template.md with the following content

Please go the the `Preview` tab and select the appropriate sub-template:

* [Group A](?expand=1&template=group_a_template.md)
* [Group B](?expand=1&template=group_b_template.md)
In this way, people who open a PR interactively in the UI will first get to the default template and can open their respective target template from the "Preview" view.

Not optimal, but more convenient than patching the URL manually every time you submit a PR.

Or use the url to choose which template we want to use as github suggests it, but as @nrogerarkhn said it can be a bit overwhelming.

@nrogerarkhn
Copy link

Indeed, there is the issue of dealing with multiple templates to address. It seems that this stackoverflow thread adresses it by suggesting two non-perfect solutions :

I found a more convenient solution for this today. The problem with the current answer(s) is that you either need to create a custom URL, or create an additional template outside of the PULL_REQUEST_TEMPLATE directory.

The alternative is to use a symlink, like this:

cd .github
ln -s PULL_REQUEST_TEMPLATE/pull_request_template.md pull_request_template.md
git add pull_request_template.md
git commit -m "Add symlink to default pull request template"
Once merged into the default branch, any new pull requests created through github.com will use the linked template.

And

You can create a manual template selection, such that at least developers who open a PR can click on a link to get to their respective template:

Assuming you have two templates group_a_template.md and group_b_template.md under .github/PULL_REQUEST_TEMPLATE

Create the default template .github/pull_request_template.md with the following content

Please go the the `Preview` tab and select the appropriate sub-template:

* [Group A](?expand=1&template=group_a_template.md)
* [Group B](?expand=1&template=group_b_template.md)
In this way, people who open a PR interactively in the UI will first get to the default template and can open their respective target template from the "Preview" view.

Not optimal, but more convenient than patching the URL manually every time you submit a PR.

Or use the url to choose which template we want to use as github suggests it, but as @nrogerarkhn said it can be a bit overwhelming.

I kinda like the second solution, it's a quite elegant way to get around the problem in my opinion!

@ALidghi
Copy link
Author

ALidghi commented Aug 25, 2023

  • SQLFMT pose problème avec les fonctions FHIR et la gestion des quotes (à tester sur le codex, voir le retirer de la PR du template)
  • SQLFLUFF déjà sur le pre-commit, à retirer du template PR
  • gérer le doublon de template de PR (tech/data)
  • Formulation à reprendre, partager le fardeau
  • comparer les pre-commit déjà implémenter dans le codex et ce template de PR : https://github.com/arkhn/codex/blob/main/.github/workflows/pre-commit.yml

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.

[PR Data] Create a git template#61
4 participants