Suggestion: We should avoid merge commits.

[shailesh-vaidya]

Just a suggestion ☺️
https://github.com/Seagate/cortx shows merge commits in history and its bit difficult to associate commit with task/ticket. We can merge PR using Squash and merge option with proper description. So that we can identify changes done for particular ticket/task.

Current git history graph looks like this,

[root@ssc-vm-0951 cortx]# git log --decorate --oneline --graph
*   21e5e49 (HEAD, main) Merge pull request #263 from Seagate/Saumya-Sunder-Contribution-Guide
|\
| *   9b7cb02 Merge branch 'main' into Saumya-Sunder-Contribution-Guide
| |\
| |/
|/|
* |   1aa39a0 Merge pull request #282 from Seagate/justinzw-patch-6
|\ \
| * \   6cb2816 Merge branch 'main' into justinzw-patch-6
| |\ \
| |/ /
|/| |
* | |   3cc38af Merge pull request #285 from Seagate/novium258-patch-1
|\ \ \
| * \ \   292fc6b Merge branch 'main' into novium258-patch-1
| |\ \ \
| |/ / /
|/| | |
* | | |   cee4f5b Merge pull request #301 from Seagate/Saumya-Sunder-patch-3
|\ \ \ \
| * \ \ \   d4fade9 Merge branch 'main' into Saumya-Sunder-patch-3
| |\ \ \ \
| |/ / / /
|/| | | |
* | | | |   92443ef Merge pull request #316 from Seagate/add-submodules
|\ \ \ \ \
| * | | | | f469fa9 refactor: add cortx-manager and cortx-management-portal as submodules
* | | | | |   3f09a00 Merge pull request #317 from Seagate/Updates-on-19
|\ \ \ \ \ \
| |/ / / / /

[huanghua78]

Hello Shailesh, I also suggested to do "Squash and merge" for every PR. Also the commit messages must contain what this particular commit does - a brief and meaningful message. Best Regards, Huang HUa

…

________________________________ From: Shailesh Vadiya <[email protected]> Sent: Sunday, September 20, 2020 9:27 PM To: Seagate/cortx <[email protected]> Cc: Subscribed <[email protected]> Subject: [Seagate/cortx] Suggestion: We should avoid merge commits. (#321) Just a suggestion ☺️ https://github.com/Seagate/cortx<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Seagate_cortx&d=DwMFaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=bNhabQwE5LsoO-JZ8kBAH4UPloa28dvCIUdNXKvzek0&m=Djf2HnqYwoRVmmIyhbQPtBpJ_yUU_pn1Hdzn1M2vwAE&s=MxmgNlyUnwGI4jP_Vg3DehAmlv26WgOllvC7SGAnDHY&e=> shows merge commits in history and its bit difficult to associate commit with task/ticket. We can merge PR using Squash and merge option with proper description. So that we can identify changes done for particular ticket/task. Current git history graph looks like this, [root@ssc-vm-0951 cortx]# git log --decorate --oneline --graph * 21e5e49 (HEAD, main) Merge pull request #263 from Seagate/Saumya-Sunder-Contribution-Guide |\ | * 9b7cb02 Merge branch 'main' into Saumya-Sunder-Contribution-Guide | |\ | |/ |/| * | 1aa39a0 Merge pull request #282 from Seagate/justinzw-patch-6 |\ \ | * \ 6cb2816 Merge branch 'main' into justinzw-patch-6 | |\ \ | |/ / |/| | * | | 3cc38af Merge pull request #285 from Seagate/novium258-patch-1 |\ \ \ | * \ \ 292fc6b Merge branch 'main' into novium258-patch-1 | |\ \ \ | |/ / / |/| | | * | | | cee4f5b Merge pull request #301 from Seagate/Saumya-Sunder-patch-3 |\ \ \ \ | * \ \ \ d4fade9 Merge branch 'main' into Saumya-Sunder-patch-3 | |\ \ \ \ | |/ / / / |/| | | | * | | | | 92443ef Merge pull request #316 from Seagate/add-submodules |\ \ \ \ \ | * | | | | f469fa9 refactor: add cortx-manager and cortx-management-portal as submodules * | | | | | 3f09a00 Merge pull request #317 from Seagate/Updates-on-19 |\ \ \ \ \ \ | |/ / / / / — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Seagate_cortx_issues_321&d=DwMFaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=bNhabQwE5LsoO-JZ8kBAH4UPloa28dvCIUdNXKvzek0&m=Djf2HnqYwoRVmmIyhbQPtBpJ_yUU_pn1Hdzn1M2vwAE&s=mDXHhkxbzaj5denSq6uLEuBrEYtjnzsafiD0U5ce_LA&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AH27YQSL63KEPBLKIKAIB4DSGX7NLANCNFSM4RTUNVUA&d=DwMFaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=bNhabQwE5LsoO-JZ8kBAH4UPloa28dvCIUdNXKvzek0&m=Djf2HnqYwoRVmmIyhbQPtBpJ_yUU_pn1Hdzn1M2vwAE&s=xhrmKgo0TgaxvTIe5sHRulQwFoJfRVIri2Ru-5j3JhQ&e=>.

[justinzw]

@shailesh-vaidya @huanghua78

@johnbent and I moved this to our brand new github discussions area.

I think this is a good subject to "discuss" here.

[shailesh-vaidya]

We can make it strict by disabling Allow merge commits in repository settings 😃

[mukul-seagate11]

We can make it strict by disabling Allow merge commits in repository settings 😃

Thanks, as this is already initiated

[shailesh-vaidya]

@justinzw @mukul-seagate11

Currently we have 76 branches in https://github.com/Seagate/cortx 😟

[root@localhost cortx]# git branch -r | wc -l
76
[root@localhost  cortx]#

Seems we are not deleting branches after merge. Ideally we should move to fork -> change -> PR workflow so branches will not be created in parent repo.
For branch base workflow, branch should be deleted after merge. We can also automate this by enabling Automatically delete head branches in https://github.com/Seagate/cortx-re/settings

[mukul-seagate11]

@justinzw @mukul-seagate11

Currently we have 76 branches in https://github.com/Seagate/cortx 😟

[root@localhost cortx]# git branch -r | wc -l
76
[root@localhost  cortx]#

Seems we are not deleting branches after merge. Ideally we should move to fork -> change -> PR workflow so branches will not be created in parent repo.
For branch base workflow, branch should be deleted after merge. We can also automate this by enabling Automatically delete head branches in https://github.com/Seagate/cortx-re/settings

yes sounds good & should be implemented but before automating this process we have to discuss/notify all components

[shailesh-vaidya]

@mukul-seagate11 Can we at least remove stale and merged branches from cortx repo - https://github.com/Seagate/cortx/branches/stale

[mukul-seagate11]

@mukul-seagate11 Can we at least remove stale and merged branches from cortx repo - https://github.com/Seagate/cortx/branches/stale

yes we have to plan to remove the stale branches

[mukul-seagate11]

@shailesh-vaidya, We had enabled auto delete branches from cortx repo once they are merged as well as will initiate to delete the stale branches as well