[platform-ui-dev] Commit conventions

["Andrey Loskutov" <loskutov@xxxxxx>]

Hi,

I wanted to write this mail since I've started to contribute to Plattform UI but never got time.

TL;DR: I would like to change Platform UI commit conventions.

Current guideline enforces me doing things which I do not like to do because their are against common sense and common  programming / contributing principles.

The Platform UI commit guideline itself (see [1]) says almost nothing (just gives this example):

############
Bug XXXXXX - Bug Title as in Bugzilla

Short description of what the fix contains, or the direction of the fix
Change-Id: I0000000000000000000000000000000000000000
Signed-off-by: email-with-CLA
############

There are important problems I have with this guideline:
1) If I need multiple commits for one bug I'm enforced to use same commit subject. WHY?
2) If I want to have a different commit subject, remove an unnecessary import or reformat few lines of code I have to create a bug. WHY?
3) I don't like commit subjects telling me nothing at all about the actual change (bug title is NOT the change).

Let's see by example how the result looks like.

Currently commits messages written according to our commit message guideline results in the history or gerrit queue like below (I've used [2], [3]):

Bug 481608 - Remove unused methods in Activators
... 4 more of same "Remove" commits...
Bug 471782 - [ViewMgmt] History View always blank
Bug 471782 - [ViewMgmt] History View always blank
Bug 478335 - Remove dead code and commented out code
Bug 478433 - Minor formatting fixes
Bug 478433 - Minor formatting fixes
...
Bug 478673 - Move platform text to Java 1.7 BREE
... 20 more of "Move" ...

Quickly scan through the changes and try to *guess* a commit changed IDEWorkbenchPlugin or EModelService? No chance. Also, on first glance, the History View was changed twice (it is not!) and ~20 attempts were made to move platform text to Java 1.7 BREE. WHAT?

I would say this commit history is not really useful or helpful.

1) Discoverability of changes in Platform UI is very poor. We have many commits with identical subjects and with subjects absolutely unrelated to the actual change.
2) Bug title usually describes a problem, but commit subject should point to the solution, which can be completely unrelated to the observed bug (like the one with History View).
3) Bureaucracy: we are enforced to create bugs for trivial things like code formatting. I really feel the pain to create a bug and spam everyone multiple times just to remove unneeded import. Why??? I can better spend my time doing more interesting stuff (for example contribute to another project). A bug entry about "remove white space" does not help anyone, there are no stack traces to attach or screenshots to share, no IP check must be done etc. For such trivial changes a commit subject should be enough.

Actually I found that I'm contributing more to EGit as to Platform UI *also* because it is more attractive, less boring for me and because in EGit I'm not acting against my own  programming skills and principles.

So can we do better? Compare the list above with egit commits list (from [4]):

Fix path prefix computation in RepositoriesViewCommandHandler
Enable diff on double-click from staged area for non-workspace files
Change label in context menu on staged StagingEntry
Follow JGit, use org.eclipse.jgit.annotations.Nullable in EGit too
Monitor Eclipse editors changing workspace external text files
Unify "Open Editor" implementation and behavior for external files
Allow to open external files in the editor directly from Staging view
Make diff from staging view also work for files not in the workspace
Replaced CommonUtils.getAdapter*() methods with AdapterUtils.adapter()
Added adapters to IURIEditorInput to better support external files
Implement isSynchronized() in LocalNonWorkspaceTypedElement

Do you see/feel the difference? (and yes, the commits *are* connected to bugs in bugzilla, you just don't see it in the commit *subject*).
The commit guidelines in EGit/JGit (see [5]) are much longer and more explicit but they make much more sense for me. 

What is the main point there? (short summary from me):
1) The first line (subject) should be a clear and concise description about the actual change.
2) *If* there is an associated bug number in Bugzilla about it, it should come as a "Bug:" footer.
3) A bug is not needed for commit, but a meaningful description of the change content is mandatory.

Example from [5]:
############
Fix the commit dialog to respect the workbench's selection

Originally, the commit dialog would automatically check off all
files in the dialog. This behaviour contradicts a user's expectation
because their selection in the workbench is completely ignored. The
code has been corrected to only preselect what the user has actually
selected.

Bug: 12345
CQ: 6031
Change-Id: I71ac4844ab9d2f848352eba9252090c586b4146a
Signed-off-by: Your Name <your.email@xxxxxxxxxxx>
############

So this was a long introduction but now I would finally like to open the discussion about the proposal below regarding commit guidelines change for Platform UI. 

The proposal is short: change Platform UI commit guidelines to follow guidelines from EGit/JGit, see [5].

This would imply:
1) A commit subject (first line) must not repeat bugzilla bug title but must be a concise change summary.
2) Trivial commits do not require to point to any bug, or other way around: commits without associated bug are possible.
3) Bug: <number> footer line is used instead of the "<Bug number and title from bugzilla>" subject to allow gerrit connect commits to bugzilla.

WDYT?

[1] https://wiki.eclipse.org/Platform_UI/How_to_Contribute#Commit_Message_Conventions
[2] https://git.eclipse.org/c/platform/eclipse.platform.ui.git/log/
[3] https://git.eclipse.org/r/#/q/status:open+project:platform/eclipse.platform.text,n,z
[4] http://git.eclipse.org/c/egit/egit.git/log/
[5] https://wiki.eclipse.org/EGit/Contributor_Guide#Commit_message_guidelines

P.S:
Thanks for reading this long mail.

Kind regards,
Andrey Loskutov

http://google.com/+AndreyLoskutov