Pull Request Requirements #1058
Conversation
Signed-off-by: Angelo De Caro <[email protected]>
adecaro
requested review from
barvhaim,
aaadir,
AkramBitar and
alexandrosfilios
DEVELOPMENT.md
Outdated
|
|
||
| - **Description**: Every pull request must include a meaningful description outlining the purpose and scope of the change. | ||
| - **Labels**: Assign appropriate labels to help with categorization and prioritization. | ||
| - **Project Assignment**: The pull request must be added to the **Token-SDK Project** board with an appropriate status (e.g., “To Do”, “In Progress”, “In Review”, etc.). |
There was a problem hiding this comment.
the name actually appears as "Fabric Token SDK"
| @@ -72,7 +72,20 @@ We follow a **linear commit history** to keep the Git log clean and easy to foll | |||
| - They make it harder to identify what changes were made. | |||
| - They complicate tools that generate changelogs or audit logs. | |||
|
|
|||
| ## 3. Additional Notes | |||
| ## 3. Pull Request Requirements | |||
There was a problem hiding this comment.
can you please also mention the PR review process? E.g. is it enough that one of the reviewers approve or must all of them do?
- introduced One Approve Policy Signed-off-by: Angelo De Caro <[email protected]>
|
|
||
| This ensures that all contributions are tracked, visible on the project board, and aligned with the roadmap. | ||
|
|
||
| ## 4. Additional Notes | ||
|
|
||
| - Use clear, concise commit messages (preferably using [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/)). | ||
| - Squash commits as needed before submission to keep the history clean. |
There was a problem hiding this comment.
- Do we need to do the Squash before rebasing? What is the recommendation?
- The default option in the PR ("Squash and Merge") can we change the default to be "Rebase and Merge"?
There was a problem hiding this comment.
Good points:
- No, you don't need to squash all your commits before merging into main. The recommendation is to squash commits that have the same message, meaning that they belong to same small unit of changes.
- I have disabled the
Squash and Mergeoption.
There was a problem hiding this comment.
During development we make many small commits/fixes. It makes sense to squash these before merging so that we have less commits. However, ideally these intermediate commits will all be functional versions (the CI will work).
- addeded a note about branch naming Signed-off-by: Angelo De Caro <[email protected]>
|
@AkramBitar , please, let me know if the changes are okay. |
|
@adecaro LGTM. One last question. Do we need to remove the branch after merging the PR to main? Is that nice to have or must, if must then could you please update the doc (also, in general do we need to remove unused branches) |
| - **Labels**: Assign appropriate labels to help with categorization and prioritization. | ||
| - **Project Assignment**: The pull request must be added to the **Fabric Token SDK** project with an appropriate status (e.g., “To Do”, “In Progress”, “In Review”, etc.). | ||
| - **Associated Issue**: A GitHub Issue must be linked to the pull request. | ||
| This should be done via the Development section of the GitHub PR interface (not just mentioned in the description). |
There was a problem hiding this comment.
Test changes also in dependent projects maybe?
|
|
||
| This ensures that all contributions are tracked, visible on the project board, and aligned with the roadmap. | ||
|
|
||
| ## 4. Additional Notes | ||
|
|
||
| - Use clear, concise commit messages (preferably using [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/)). | ||
| - Squash commits as needed before submission to keep the history clean. |
There was a problem hiding this comment.
During development we make many small commits/fixes. It makes sense to squash these before merging so that we have less commits. However, ideally these intermediate commits will all be functional versions (the CI will work).
This PR introduces the
Pull Request Requirementssection in theDEVELOPMENT.mdfile. The purpose is to give instructions to the developers on how to prepare a pull request.