Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [platform-ui-dev] Commit conventions

Hi Andrey,

I think this is not a Platform UI specific issue, it is the convention
for the whole platform and / or for the whole top-level Eclipse
project. I think, if we want to simplify that, we need to change the
rules for the top-level project, as more and more people work
cross-project.

Dani as project lead for the Eclipse platform and Eclipse top-level
project would need to decide that. Or bring it to the Eclipse
top-level PMC.

A weak +1 from my side for this simplification. If you and others
suffer from this, I'm for changing it but I personally do not suffer
from it, hence the "weak" part of my vote.

Best regards, Lars

On Mon, Nov 16, 2015 at 3:40 PM, Andrey Loskutov <loskutov@xxxxxx> wrote:
> 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
> _______________________________________________
> platform-ui-dev mailing list
> platform-ui-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/platform-ui-dev



-- 
Eclipse Platform UI and e4 project co-lead
CEO vogella GmbH

Haindaalwisch 17a, 22395 Hamburg
Amtsgericht Hamburg: HRB 127058
Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
USt-IdNr.: DE284122352
Fax (032) 221739404, Email: lars.vogel@xxxxxxxxxxx, Web: http://www.vogella.com


Back to the top