Commenting every line or block of code change with the task number of the change.



  • Opinions?

    I don't want to sound crazy when I say I don't like this.



  • Stupid idea... unless it's some hack or WTF you want to justify... and continuing with the eval topic:

    // Because of TICKET-14 and stupid shit
    var ret = eval(input);


  • I feel that way.

    // ticket5030
    code line 1
    // ticket5040
    code line 2 // ticket5050
    // end ticket 5030
    code line 3
    ...
    code line 10
    //end ticket 5040
    


  • But, I need a reason for why it's stupid.

    Like, a concrete reason.

    I'm thinking it's clutter.

    But I'm told it's for quickly finding out what feature the new lines are introducing.



  • Well, because comments are usually hard to maintain:

    // Because of TICKET-14 and stupid shit
    var ret = eval(input);
    

    On next sprint

    // Because of TICKET-14 and stupid shit
    try{
        // Fix TICKET-27
        var ret = eval(input);
    }catch(err){
        // w00t!
    }
    

    And on next sprint

    // Because of TICKET-14 and stupid shit
    try{
        // Fix TICKET-27
        var ret = eval(input);
    }catch(err){
        // Fix Ticket-29 handle errors
        handleError(err);
    }

  • I survived the hour long Uno hand

    Sane people put the ticket numbers in the commit message, so you can find them when doing a blame but they don't clutter up the code.



  • It makes sense to me IF the ticket tracking software is suitably easy to navigate and your fellow developers actually use the support tickets to track and discuss the bug and fix, instead of using email chains



  • I've wondered the same thing.

    It's in the source control, that's what that's for.

    We put the ticket/task in the source control commits too.



  • They're stupid because any decent developer uses a source control system to keep track of changes, and any decent source control system offers a "Blame" feature to highlight who changed what line of code at a given moment. In the change, just mention the ticket number. In TFS you can also link a work item (such as a Task or Bug) so that all specifics about why the change was made is within a few clicks reach.

    Ticket numbers in code is just useless clutter that nobody reads (let alone understands) and that is maintained even less, as explained above.


  • ♿ (Parody)

    @mrguyorama said:

    It makes sense to me IF the ticket tracking software is suitably easy to navigate and your fellow developers actually use the support tickets to track and discuss the bug and fix, instead of using email chains

    Sufficiently advanced WTFery is indistinguishable from sabotage.



  • svn/git blame/annotate might work. Avoids the hassle of adding it in comments, and can retrieve it for every line.



  • Going back to iInesat, I used put details of changes in the "commit" message. I was told to stop doing that (for some obscure reason that escapes me now but had something to do with not being necessary because "...if you type git -umpteen [stupid/switches], it will list all the changed code, besides you can use bugzilla..."). Oh, god! don't get me started on that - ALL "on the fly / ad hoc" documentation was it's own seperate incarnation of bugzilla.

    Getting back on topic: When I bugfix / feature enhance old spaghetti code I like to leave something like a ticket or reference number - mostly because a single simple fix would often involve several changes in and across several "scripts". Also, after "decoding" something that turned out not to be the issue, I would leave a note or reference for use when it did become the issue. Or I was (eventually) given a refactoring shotgun license.

    Sometimes I leave the old code there, just in case...

    When I am refactoring, I use self-documenting code :rofl:, or rather code that has meaningful names and appropriate comments etc.



  • The source control merges should have the task numbers.

    There's no need for the source code itself to have them.

    If you want to know the number, view the file's history and yank it from source control.

    THUS IS MY OPINION.


  • Winner of the 2016 Presidential Election

    IMO it depends on a few things:

    Did that already happen to the rest of the code? If so: Not crazy (at least not more crazy than the other people who thought the exact same thing)
    Do you really mean EVERY LINE? If so: crazy (IMO, see point 1)
    Do you think it would create too many comments? If so: crazy because of the reasons other people have pointed out here.
    Do you lose your job if you don't comment every line? If so: crazy, but you should probably still go job hunting!

    Filed Under: Just my two cents as a guy who never had to face this problem



  • @Kuro said:

    Did that already happen to the rest of the code?

    Non consistently, but now that it is in the review document there is more pressure to see it done.



  • @xaade said:

    But, I need a reason for why it's stupid.

    Like, a concrete reason.

    Because your VCS should be doing that for you. If you use TFS, you can actually hook tickets to changesets - don't know if any other system provides that level of integration, but there's always commit messages.

    No need to comment each line, and a year from now, who gives a shit which ancient ticket was resolved by that line?

    Also what do you do when you change the code?

    //resolves ticket 57
    //now resolves ticket 95 - Dave
    //actually it resolves both, and also 215 - Joe
    

  • Java Dev

    @Yamikuronue said:

    Sane people put the ticket numbers in the commit message, so you can find them when doing a blame but they don't clutter up the code.

    This. The only reason to put a ticket number in a comment is if you're doing something particularly non-obvious, and linking to the ticket for background saves at least 5-10 lines of comment.



  • We are using TFS....

    @Maciejasjmj said:

    //resolves ticket 57
    //now resolves ticket 95 - Dave
    //actually it resolves both, and also 215 - Joe

    yep.


  • I survived the hour long Uno hand

    @Maciejasjmj said:

    If you use TFS, you can actually hook tickets to changesets

    JIRA and Github both display relevant commits on the ticket if you put the ticket number in the commit message.



  • @xaade said:

    Non consistently, but now that it is in the review document there is more pressure to see it done.

    So some moron decided to enforce this and leave their stupidity in printed text? HA!

    @Yamikuronue said:

    JIRA and Github both display relevant commits on the ticket if you put the ticket number in the commit message.

    Yep, and are smart enough to know that this:

    Fix TICKET-17

    Means that the ticket is fixed and reset its status.

    And if you use pull requests you also get this:



  • The review document is an index of topics to review in code.
    As such, a line item is these code markers.



  • @xaade said:

    Like, a concrete reason.

    1. Because it will not reflect reality for long.
    2. Because not all tickets are resolved by adding lines of code.
    3. Because the effort to maintain it is disproportionate to its utility.
    4. Because there is better place for this information, the version control system.

    Ad 1:

    Humans are not reliable. You can check it, but sometimes somebody will still forget to add the reference or make a typo in it and the information will gradually become unreliable. If it is not reliable, nobody will use it. If nobody will use it, there is no point wasting the effort to produce it. Because it is effort.

    Ad 2:

    Ok, so let's say whenever you add code, you annotate it with the link and you are careful enough that the information remains reliable. But most bugs are not fixed by adding code. Most bugs are fixed by changing code. Now what will you do with the comments for previous lines. Should you keep those references in?

    And changing a line is still the simple case. Sometimes you fix a bug by reordering two lines. Now which one fixes the bug? How do you express it?

    These problems make it a lot of effort to maintain it in useful state and even then it is not able to reflect evolution of the code very well.

    Ad 3:

    However most of the time you don't need the information. Most code will be obvious enough that you don't need to constantly look up the requirements and tickets and whatnot. Unfortunately it is not always obvious what might come back as a bit strange when you find next bug. So if you want to rely on this information being available, you must maintain it everywhere, but in most places you will not end up needing it.

    Of course there are bits that are obviously tricky and putting a reference to a ticket that one must mind when working with it makes sense. But it is occasional tricky bit, not all over the place.

    Ad 4:

    As already mentioned by others, the version control system is a better place for this kind of information. Your commit or merge notes should include

    1. references to tickets the change is supposed to resolve,
    2. brief explanation of the cause of the problem and
    3. explanation how and why the change fixes it.

    Most tickets describe symptoms, but those are usually not much help when trying to understand the code. You need a glimpse of the mental process of the person who touched it before you. So unless you put technical analysis in the tickets—and I have not seen many programmers who do, not least because the tickets are usually communication channel to testers or even customers and those couldn't care less—you should put it somewhere and that somewhere is the commit comment. It is a good place for it. You are writing it when you still have fresh memory of why you wrote what you just did and it will remain tied to the particular state you are leaving it in, so if someone needs to analyse the code later, they can reconstruct its evolution which does not work so well from comments.

    All version control systems have tools to dig up relevant comments when desired (blame, log, pickaxe…). The effort of using them if you need to analyse particularly difficult bit of logic is less then the effort of maintaining comments in useful state everywhere and if you wrote sensible commit logs gives better results.



  • @xaade said:

    Like, a concrete reason.

    Anyone who sees a need to put a "ticket XYZ" comment in code has never used a decent source code control system. It's a sign of professional immaturity and/or fear of anything invented this century.

    The best way I can think of to explain it concretely is that it solves a problem that you shouldn't have if you use source code control. Comments should explain the code, not the history of how it became the way it is.



  • @Jaime said:

    It's a sign of professional immaturity

    Oops, well in my defense, it was within the first like month of my professional life. I shall avoid doing this in the future, especially since our svn repo REQUIRES defect codes



  • BTW, I chose the phrase "professional immaturity" purposely in an attempt to convey the lack of exposure/experience without giving a connotation of not being talented. I hope you weren't offended as that was not my intention.



  • Nothing wrong with spitting a little vinegar every now and then. Especially here on TDWTF



  • It's not terrible, if it's just a block, as opposed to every line. But at the same time, that data is already in your VCS logs, assuming you mark what ticket number you were working on in the logs.

    It can definitely fit with a "Why" when commenting a block of code, but I don't know if every single change warrants it.


  • Fake News

    @AlexMedia said:

    They're stupid because any decent developer uses a source control system to keep track of changes

    I had a (senior) colleague who did put such comments in the code. His reason: "I once had to work at a customer where they lost all commit messages when they migrated the VCS; with these comments, we wouldn't have have lost that information".

    Might have been management material... Anyhow, another datapoint for @Jaime.



  • If ticket numbers belong in comments, so does superseded code. Both of these are so much easier for managers than learning to use a VCS!



  • Another reason: not all fixes include a single line of code. Some fixes require rewriting a whole module or several modules.


Log in to reply