AI PR adds auto generated comments to whole Spring Boot Project

by sondr3on 2/27/24, 11:47 AMwith 41 comments
by Someone1234on 2/27/24, 4:22 PM

Comments that say HOW commonly useless, as that is almost always self-evident (i.e. "just read the code."). Comments that explain WHY are worth their weight in gold, and not something AI could ever create.

For example:

    // How: This filters records by MyType of J and if they have children   
    // Why: We want Jobs (J) when those Jobs have Children. Otherwise, we'll get JobGroups or Jobs without any Children we need to process below.   
     .Where(T => T.MyType == "J" && T.Child.Count() > 0);  
WHY are, by their very nature, business rules or explaining a thought process. If the original developer of the code moves on or is unavailable, that information can sometimes not be found elsewhere.

by timetraveller26on 2/27/24, 4:01 PM

As said in the PR comments, this is Codemaker AI advertisement, no good will.

And funnily enough, bad advertisement. How could a startup that focuses on code think that making a PR with 3,158 changed files is acceptable?

by preommron 2/27/24, 4:31 PM

Ironically, it would've been more useful, and more inline with how non-trivial advancements in AI have been, to identify useless and obvious comments, and eliminate them.

I just experimented a little bit, and telling chatgpt to assume a self-documenting approach and skipping comments whenever possible, gets a pretty minimalist output. Add on asking about what non-obvious aspects might be noteworthy based on other projects, and I bet chatgpt would be able to find a list of things to look for, see if it's applicable, and then rewrite in the appropriate format. Like if the function is about sorting, it could figure out that knowing if it's stable is an important marker, and figure that out. Something like that could be useful.

by lelandfeon 2/27/24, 3:51 PM

Given that all the comments I'm seeing are of the “‘makeFooReady’ makes foo ready” ilk, I wonder how these tools fare on incorrectly-named things. Would it pick up on ‘asyncRemoveFoo’ being neither async nor a function that removes foo?

by mateusfreiraon 2/27/24, 3:43 PM

I struggle to see how any open-source project would accept this kind os PRs, the tools seems ok for research, but not a good ideia to push PRs like that open source projects.

by ptxon 2/27/24, 4:26 PM

I'm curious what the PR description means by "the tool reached a compilation success rate of 99.9%". Did it break the code and introduce compilation failures when adding the useless comments?

by shadowgovton 2/27/24, 3:51 PM

To be honest, I see two positives regarding what Codemaker has provided here.

One is that (glancing through the comments) they're actually significantly more thorough than what is already there and they're at the level of thoroughness that a new user may actually want. Phil Webb is quite right; this is the level of detail someone new to the codebase could use, and if you could generate it dynamically on the fly as a "Help me understand what I'm looking at" tool that'd be really nice-to-have.

Second: I've definitely worked at places so tight-ass about code documentation that they do want "makeAppleRed(): Makes the apple red." Mostly because they're using doc engines that do a bad job of letting you know a function exists at all if it isn't documented. I have no doubt Codemaker is going to make its money on those places.

by cjon 2/27/24, 3:58 PM

This sort of AI-generated code context seems like it would be better as a VS Code extension rather than in code.

by TrianguloYon 2/27/24, 4:45 PM

Serious questions: for those of you who say "just read the code", don't you find it useful when your IDE autocompletes what a function does? I do. And sometimes you can't even read the code!

Note: I'm not talking about things that you know just by looking at the method itself, but things that are obvious when reading the code of the method only.

    /** This will return the data */
    Data getData() {...}
This is useless, But

    /** This will return true iff it's not empty */
    boolean isValid() {return !empty();}
This is useful. This is what a good documentation should provide, and having documentation in the code itself (that you can later extract to an html page or other) is way better than having it on a separate platform that you need to remember to update.

Edit: remember that this is library that other people will use, it's not an internal tool that only your team knows about.