Adding a PR template - Githubissues

tenstorrent / tt-mlir

Tenstorrent MLIR compiler

https://tenstorrent.github.io/tt-mlir/

Apache License 2.0

51 stars 7 forks source link

Adding a PR template #480

Closed vmilosevic closed 1 week ago

vmilosevic commented 2 weeks ago

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.

Issue Number: (e.g. #123)

Type of a change

[ ] Bug fix
[ ] New feature
[ ] Performance improvement
[ ] Documentation update
[ ] Github Actions CI change

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so that reviewers can reproduce your tests. List any relevant details for your test configuration.

Test A: (e.g. Ran unit tests)
Test B: (e.g. Tested in staging environment)

vmilosevic commented 2 weeks ago

I agree with most details @nsmithtt already outlined. For PRs, it'll be good if they can be as simple as possible to create. I'm also more for it to keep a sanity check over our commits rather than PRs (let's try to extract commit data into PR).

@vmilosevic is it possible to extract some PR info based on commit and issue? If yes, I'm more for us to focus on maintaining clean and properly tagged issues and descriptive commits, rather than safekeeping PRs. Let us know what are possibilities here :))

While extracting commit messages is not possible, it appears that there is an option to do it the other way around, by setting PR title and description as squash commit message.

nsmithtt commented 2 weeks ago

I'm kind of leaning towards us just getting in the habit of writing good commit messages vs having a PR template. The Description section is by far the most important part and that should just be the commit message to begin with.

Same with the "how has this been tested" section, it's tested with CI and instead of writing something here we should probably just get in the habit of enforcing a unit test for every PR (with some exceptions).

nvukobratTT commented 2 weeks ago

I'm kind of leaning towards us just getting in the habit of writing good commit messages vs having a PR template. The Description section is by far the most important part and that should just be the commit message to begin with.

Same with the "how has this been tested" section, it's tested with CI and instead of writing something here we should probably just get in the habit of enforcing a unit test for every PR (with some exceptions).

I agree. We can define a "general" commit template that will mirror on the PR using the configuration @vmilosevic recommended. E.g.

Single sentence description of the change.

Details about the change, impact, linked PRs (e.g. #123), etc.

nsmithtt commented 1 week ago

It sounds like we are leaning towards git commit message best practices. Filed issue for tracking:

https://github.com/tenstorrent/tt-mlir/issues/566