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

Add draft github pull-request template #53

Merged
merged 14 commits into from
Jul 11, 2024
+55 −0
Merged

Add draft github pull-request template #53

Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
02cc3b6
Add draft github pull-request template
leifdenby Jun 3, 2024
a5ddeb1
Update .github/pull_request_template.md
leifdenby Jun 10, 2024
6f8f8a7
Update .github/pull_request_template.md
leifdenby Jun 10, 2024
46565e7
Update .github/pull_request_template.md
leifdenby Jun 10, 2024
549bd6b
Update .github/pull_request_template.md
leifdenby Jul 7, 2024
20edece
update with comments from review
leifdenby Jul 7, 2024
08604f1
Merge branch 'main' of https://github.com/mllam/neural-lam into maint…
leifdenby Jul 7, 2024
fa24eb8
Merge branch 'maint/add-github-pr-template' of https://github.com/lei…
leifdenby Jul 7, 2024
ece725f
changelog
leifdenby Jul 7, 2024
f432f0e
Update .github/pull_request_template.md
leifdenby Jul 9, 2024
bd31b96
Update .github/pull_request_template.md
leifdenby Jul 11, 2024
60ca712
Update .github/pull_request_template.md
leifdenby Jul 11, 2024
4483fd3
Update .github/pull_request_template.md
leifdenby Jul 11, 2024
1fe3348
changelog lint
leifdenby Jul 11, 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
42 changes: 42 additions & 0 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
## Describe your changes
< Summary of the changes.>

< Please also include relevant motivation and context. >

< List any dependencies that are required for this change. >
## Issue Link
< Link to the relevant issue or task. > (e.g. `closes #00` or `solves #00`)
## Type of change

- [ ] 🐛 Bug fix (non-breaking change that fixes an issue)
- [ ] ✨ New feature (non-breaking change that adds functionality)
- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)

## Checklist before requesting a review

- [ ] My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use `pull` with `--rebase` option if possible).
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] I have requested a reviewer and an assignee (assignee is responsible for merging)
- [ ] I have given the PR a name that clearly describes the change, written in imperative form ([context](https://www.gitkraken.com/learn/git/best-practices/git-commit-message#using-imperative-verb-form).
## Checklist for reviewers

Each PR comes with its own improvements and flaws. The reviewer should check the following:
- [ ] the code readable
- [ ] the code well tested
- [ ] the code documented
- [ ] the code easy to maintain
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- [ ] the code readable
- [ ] the code well tested
- [ ] the code documented
- [ ] the code easy to maintain
- [ ] the code is readable
- [ ] the code is well tested
- [ ] the code is documented
- [ ] the code is easy to maintain

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it clear how to test "easy to maintain"? Maybe we should be a bit more specific here (e.g. modular, using existing open-source libraries, ...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think most of these can be expanded on and made more concrete. But that then turns into more of a coding style guide, that probably fits better somewhere else (related to my other comment).

Copy link
Member Author

Choose a reason for hiding this comment

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

we can also leave "easy to maintain" out? The other three cover this some degree and this isn't explicit enough about how we measure this that the comment is probably superfluous

Copy link
Member Author

Choose a reason for hiding this comment

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

I will remove "easy to maintain" since that is hard to measure objectively, we can always adapt this template later if we find it is missing something or doesn't quite work :)


## Author checklist after completed review

- [ ] I have added a line to the CHANGELOG describing this change (in section
reflecting type of change, for example "bug fixes", add section where
missing)

## Checklist for assignee

- [ ] PR is up to date with the base branch
- [ ] the tests passing
- [ ] author has added an entry to the changelog (and designated the change as *added*, *changed* or *fixed*)
- Once the PR ready to be merged, squash commits and merge the PR.
Loading