Bug 90834 - Turn in-line version control history comments into meaningful good comments
Summary: Turn in-line version control history comments into meaningful good comments
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.5.0.0.alpha0+ Master
Hardware: Other All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0 target:5.3.0 target:6.1.0
Keywords: difficultyBeginner, easyHack, needsDevEval, skillCpp
Depends on:
Blocks: Source
  Show dependency treegraph
 
Reported: 2015-04-24 13:55 UTC by Tor Lillqvist
Modified: 2018-12-11 09:59 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tor Lillqvist 2015-04-24 13:55:26 UTC
Especially in sw we have lots of comments, mainly by a person called "OD", but possibly also others, inherited from OpenOffice.org times, that go like this:

                    // OD 06.08.2002 #99657# - add 6th parameter to indicate, if
                    //     background transparency have to be considered
                    //     Set missing 5th parameter to the default value GRFNUM_NO
                    //         - see declaration in /core/inc/frmtool.hxx.

These basically are in-line version control comments: They tell what change was done to the code at that date, and by whom. That is not information that should be in the source code (but recorded in the version control system). When possible, such comments should be changed to tell what the code does, using the present tense. The above could turn into something like:

                    // The 6th parameter indicates if
                    // background transparency have to be considered.
                    // See declaration in /core/inc/frmtool.hxx.

In other cases, like:

    // OD 05.09.2002 #102912#
    // delete temporary background brush.
    delete pTmpBackBrush;

it's just the timestamp and bug number that should be removed. They are not interesting.

    // delete temporary background brush.
    delete pTmpBackBrush;

In cases like:

// --> OD #i102707#
namespace
{
...
}
// <--

the comments are not useful at all and should simply be deleted.

Obviously exactly what to do with these "OD" comments is a matter of taste... If you plan to work on it, please submit an initial suggestion with perhaps a dozen or so comments handled for review, before wasting time on doing hundreds of cases in perhaps the "wrong" way.
Comment 1 Miklos Vajna 2015-04-24 14:15:32 UTC
Confirming. Though please don't just delete these comments as-is, the OOo bugzilla references are still useful, e.g. changing

// --> OD #i102707#
// <--

to a single

// i#102707

is what I would suggest. Names, dates, arrows can go. Also #12345# bug numbers refer to the lost stardiv bugzilla, so those can go, too.
Comment 2 Commit Notification 2015-10-08 20:47:50 UTC
kripton committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7fccad27c97d256c0def7b51e9947f13d5c9c977

tdf#90834 Turn in-line version control history comments.

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 3 Robinson Tryon (qubit) 2015-12-14 07:02:50 UTC Comment hidden (obsolete)
Comment 4 Robinson Tryon (qubit) 2016-02-18 14:52:37 UTC Comment hidden (obsolete)
Comment 5 Commit Notification 2016-03-20 11:49:38 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bcfff3e9fa4609c7f915c14de40eef80181cf815

tdf#90834 Turn in-line version control history comments to good comments.

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2016-03-20 12:55:55 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e8aea8fd732763f0b808c88debcc99522672fbd8

tdf#90834: Drop an 'in-line change log' style comment

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Muhammet Kara 2016-03-20 19:31:51 UTC
I intend to keep working on this bug for a (short) while so that I'll be warming up, and getting used to the environment/tools.
Comment 8 Tor Lillqvist 2016-03-20 19:46:59 UTC
Muhammad, good good!

yeliz, I assume you aren't working on this any longer, you assigned the bug to yourself in October, but did just one commit. For bugs like this, that many people can work on, as there are dozens or hundreds of separate changes that can be done, assigning it to one person is not useful.
Comment 9 Tor Lillqvist 2016-03-20 19:47:21 UTC
Ah, sorry, autocorrect "fixed" your first name;)
Comment 10 Commit Notification 2016-03-22 16:32:23 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=da966b5e49e49beb29be5fbdb092ad36dc7fa0ff

tdf#90834 Cleanup in-line version control history comments: widorp.cxx

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2016-03-23 17:35:37 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8e9658b43153f20f644c053cf7ffe5c7883dd14d

tdf#90834 Cleanup in-line version control history comments

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2016-04-01 06:25:53 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=20f98f52392504666c467417558fdf93e3c16c27

tdf#90834 Cleanup in-line version control history comments

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2016-04-01 06:28:30 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=403793990c1258db5b2bc4f709e6028a3d5ea2cb

tdf#90834 Cleanup in-line version control history comments

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2016-06-02 15:58:01 UTC
krishna keshav committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=66c41718f91dcca9c487742e9313c657ede4ce3f

tdf#90834 Turn in-line version control history comments

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2016-06-10 08:27:09 UTC
apurvapriyadarshi committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8970f569f1309d16d3686c75925810c94504f891

tdf#90834 Turn in-line version control history comments

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 jani 2016-06-14 10:01:36 UTC
Controlled, still open
Comment 17 Commit Notification 2016-06-20 05:23:15 UTC
apurvapriyadarshi committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=ef5b37726335034de44e6e7cc2035f798d66f753

tdf#90834 Turn in-line version control

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Commit Notification 2016-07-05 06:16:01 UTC
krishna keshav committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9b95f149b2a98df33710d5caf460575d8ee6ab52

tdf#90834 Turn in-line version control history comments

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 jani 2016-08-05 09:21:03 UTC
Please be aware, that this easyhack is considered an important but large scale cosmetic change as described in https://wiki.documentfoundation.org/Development/LargeScaleChanges

It was in decided by the ESC to close this kind of easyhacks, and send them directly as mail, to new contributors.
https://lists.freedesktop.org/archives/libreoffice/2016-August/074920.html

Please do not submit patches with many files !!

This easyhacks should/will be opened, when we have a window to do largescale changes.
Comment 20 Xisco Faulí 2016-09-27 10:36:09 UTC Comment hidden (obsolete)
Comment 21 Commit Notification 2018-04-10 05:21:45 UTC
Gökhan Gurbetoğlu committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c9292c0d370ca779a4f3351bafc8988363679dc1

tdf#90834 Cleanup version control history comments

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 Tor Lillqvist 2018-12-11 09:59:20 UTC
Please don't just bluntly assign an Easy Hack bug to yourself with no discussion or informative comment or anything.