15 ms·

AI commented the entire Spring Boot codebase

21 points by chuckhend 3y ago

oblio 3y ago
[flagged]
deleted 3y ago
[deleted]
jes5199 3y ago
I like the idea, but there’s no reason to commit these back to the repo. In fact, you could just make a code-viewer that generated fresh explanations whenever you open the file a lot of generative AI is like this. Oh, it made a beautiful picture, but why bother saving it, you can just generate a fresh one every time
- itake 3y ago
  You're not wrong, but the latency would annoy me. with 1s p99, a file with 20 functions would take 10-15s to document. Also, you can't easily search against those code comments using standard tools.
- acjohnson55 3y ago
  That would be kind of expensive though, right? Committing the comments is effectively caching the generation. Although, that caching could be done off-repo.
  - deleted 3y ago
    [deleted]
tenlp 3y ago
For a mature project, it is almost impossible to directly introduce AI and add comments to all codes.
threeseed 3y ago
Pretty funny. Especially for Java which has always tended to be a verbose language that emphasises highly descriptive class and method names. The idea is interesting but better done as an IDE plugin where I can hover over a method and have it show an auto-generated help popup. I suspect IntelliJ is already working on something like that.
pylua 3y ago
It looks like a lot of boiler plate that adds noise. Usually the most useful comments explain the function in its context, not the code itself. Or, if the code does something atypical then a comment is very useful. This is interesting, but a waste .
hombre_fatal 3y ago
The AI comments seem to get pretty helpful in the best case scenario of complicated functions, but they also get too spammy in the worst case scenario where it's adding "Returns a value" to `public getValue()`. Whether you want every function in the codebase to have a comment is a rather deliberate style, not something you'd just merge in from an unsolicited PR. But if the tool is automated and easy to use, seems like something useful for local development or a way to shop for good comments that are missing from the code.
ripjaygn 3y ago
These are exactly the kind of comments that you don't want in your code base. Comments should be about why the code is doing what it's doing. By definition, this won't be in the code, so the AI cannot divine it to existence. Readable code is about what the code is doing, which should be apparent from reading the code. This AI tool can be used as a helper for a junior dev or to follow the call stack or code flow. Maybe AI can do a good job one day when it has the full context of code, i.e requirements docs, transcripts of meetings, emails, JIRA tickets etc.
- stevage 3y ago
  >By definition, this won't be in the code, so the AI cannot divine it to existence. I don't think that's true. You're essentially saying that no one other than the original author can possibly add useful comments, and that is manifestly untrue. It's well within the reach of AI to add comments about how a function is used, and even infer things like "detects errors that might cause an exception to be thrown later".
  - ripjaygn 3y ago
    The AI is adding comments like these: private void makeAllWarningsFatal(Project project) { /** * Makes all warnings fatal for the given project. * * @param project the project for which to make warnings fatal */ What does this achieve? The code already has readable function names. It just creates clutter and makes things worse by forcing people to scroll more by making less code fit in a window. What would be useful is commenting why a setting to make all warnings fatal is a use case. For example, a comment stating that this is used to improve code quality and reduce risk by forcing the developer to fix all warnings which could be hiding errors. This AI is utterly useless at coming up with that.
    - Freedom2 3y ago
      Playing devil's (GPT's?) advocate here - they can be somewhat useful for autogenerated documentation where you don't have access to the code and only some form of browser or other text documentation. Otherwise I agree with you.
hmeh 3y ago
Given that the only comments worth having are typically those that explain why, and an AI would have a hell of a time knowing why code was the way it was, there's very little chance that this does anything but add noise. It's exactly the type of "productivity enhancement" we should expect from AI right now. That is, as long as you measure productivity by lines of code and nothing else that actually matters to the business.
splatzone 3y ago
Can someone familiar with the Spring Boot codebase confirm if these comments are useful or not? At a glance - and I'm not a java developer - a handful of the comments seem to add some specific context that's not obvious from the code alone: https://github.com/spring-projects/spring-boot/pull/39754/commits/d69317431c623f9b9be07771dba8e1d153b1e991#diff-06d556cdacae547b52a06e42d38030533e1cca0fc0b28c529f993be2d737ddaeR144 https://github.com/spring-projects/spring-boot/pull/39754/co...
- ptx 3y ago
  I'm not familiar with this codebase, but to me it doesn't seem to be adding any context – it's simply describing what the implementation does. In particular, the comment about unnecessaryExclusions is just adding noise since it's an internal implementation detail that doesn't matter to the caller.
samrolken 3y ago
Comments are most helpful when they explain something that is not obvious by just looking at the code. Comments that just explain what the code plainly does don't add much value. This kind of technology can still be useful in other forms as other comments here note, but this kind of auto-commenting, I really don't see it catching on.
stevage 3y ago
There is an awful lot of this: /\** \* Checks if a library is excluded. \* \* @param library the library to check \* @return true if the library is excluded, false otherwise \*/ protected boolean isLibraryExcluded(Library library) { return library.getName().equals("Spring Boot"); } The comment adds nothing - I'm still confused what this one line function is meant to do or what the word "excluded" means in this context. The thought definitely occurs that whatever comment value can be generated automatically could also (and perhaps should instead) be done locally for the viewer, rather than actually being stored in the codebase. Then it doesn't clutter up the codebase, and also can't get out of sync. Or since these comments are really JSDoc style comments, they could be applied just before documentation is generated.
deleted 3y ago
[deleted]
SquareWheel 3y ago
I appreciate the professional exchange between contributor and maintainer. It was contributed with the expectation not that it would be merged, but that it could be adapted into a useful PR if the maintainers were interested. They weren't, so they respectfully parted ways. Skimming through some of the code comments, this tool actually does a pretty good (though not perfect) job of interpreting each function. It would likely need to be limited to only larger functions though, since many of the smaller or boilerplate functions are self-explanatory. And seeing as this is Java, there are a lot of those!
davesque 3y ago
Seems spammy to drop a 100k line PR of just comments, AI generated or otherwise. No maintainer in their right mind would want to review that.
xyst 3y ago
Blatant advertisement for their product. Shameful pull requests like this need to be deleted/purged. Look at this trash: ... /* * Makes all warnings fatal for the given project. * @param project the project for which to make warnings fatal / private void makeAllWarningsFatal(Project project) { project.getExtensions().getByType(AsciidoctorJExtension.class).fatalWarnings("."); } ... Quite possibly the most useless javadoc I have seen. Code itself is self documenting. It's like when the developer gets paid per line of code, this is what is generated.
PaulKeeble 3y ago
I am not impressed. These comments don't add much at all. There are some that superficially look OK and to add some knowledge but in practice are wrong. Then there is stuff like this that doesn't need to be there. /** * Returns the type of the artifact release. * @return the type of the artifact release */ public String getType() { return this.type; } But then you also have the following which looks reasonably promising and does contain useful information that you would only get from the code or the docs. /** * Constructs a new instance of AutoConfigurationMetadata. * * This method retrieves the inputs and sets the file path for the * AutoConfiguration.imports file located in the * META-INF/spring/org.springframework.boot.autoconfigure directory. The path * sensitivity is set to RELATIVE and the property name is set to * "org.springframework.boot.autoconfigure.AutoConfiguration". * * The method also sets a dependency on the processResources task name of the source * set. * * Additionally, it creates a configuration named * "AutoConfigurationPlugin.AUTO_CONFIGURATION_METADATA_CONFIGURATION_NAME" if it does * not already exist in the project's configurations. */ public AutoConfigurationMetadata() { getInputs() .file((Callable<File>) () -> new File(this.sourceSet.getOutput().getResourcesDir(), @@ -68,19 +84,35 @@ public AutoConfigurationMetadata() { .maybeCreate(AutoConfigurationPlugin.AUTO_CONFIGURATION_METADATA_CONFIGURATION_NAME); }*
chuckhend 3y ago
A very kind response by someone from the project team
- rsynnott 3y ago
  Overly kind, I would say.
Jupe 3y ago
No, no, no! Tell me something useful about the code, or why it is there in the first place... Tell me where it's called from, how often it's called, if the caller can ever send null args, expected execution time, who wrote it, and when, and what else changed when it was first created, whether it is executed under a unit test or functional test. But please do not tell me what it is doing... That's the one piece of information I don't need since I'm already looking at the code.

21 points by chuckhend 3y ago

oblio 3y ago

[flagged]

deleted 3y ago

[deleted]

jes5199 3y ago

I like the idea, but there’s no reason to commit these back to the repo. In fact, you could just make a code-viewer that generated fresh explanations whenever you open the file a lot of generative AI is like this. Oh, it made a beautiful picture, but why bother saving it, you can just generate a fresh one every time

itake 3y ago
You're not wrong, but the latency would annoy me. with 1s p99, a file with 20 functions would take 10-15s to document. Also, you can't easily search against those code comments using standard tools.
acjohnson55 3y ago
That would be kind of expensive though, right? Committing the comments is effectively caching the generation. Although, that caching could be done off-repo.
- deleted 3y ago
  [deleted]

tenlp 3y ago

For a mature project, it is almost impossible to directly introduce AI and add comments to all codes.

threeseed 3y ago

Pretty funny. Especially for Java which has always tended to be a verbose language that emphasises highly descriptive class and method names. The idea is interesting but better done as an IDE plugin where I can hover over a method and have it show an auto-generated help popup. I suspect IntelliJ is already working on something like that.

pylua 3y ago

It looks like a lot of boiler plate that adds noise. Usually the most useful comments explain the function in its context, not the code itself. Or, if the code does something atypical then a comment is very useful. This is interesting, but a waste .

hombre_fatal 3y ago

The AI comments seem to get pretty helpful in the best case scenario of complicated functions, but they also get too spammy in the worst case scenario where it's adding "Returns a value" to `public getValue()`. Whether you want every function in the codebase to have a comment is a rather deliberate style, not something you'd just merge in from an unsolicited PR. But if the tool is automated and easy to use, seems like something useful for local development or a way to shop for good comments that are missing from the code.

ripjaygn 3y ago

These are exactly the kind of comments that you don't want in your code base. Comments should be about why the code is doing what it's doing. By definition, this won't be in the code, so the AI cannot divine it to existence. Readable code is about what the code is doing, which should be apparent from reading the code. This AI tool can be used as a helper for a junior dev or to follow the call stack or code flow. Maybe AI can do a good job one day when it has the full context of code, i.e requirements docs, transcripts of meetings, emails, JIRA tickets etc.

stevage 3y ago
>By definition, this won't be in the code, so the AI cannot divine it to existence. I don't think that's true. You're essentially saying that no one other than the original author can possibly add useful comments, and that is manifestly untrue. It's well within the reach of AI to add comments about how a function is used, and even infer things like "detects errors that might cause an exception to be thrown later".
- ripjaygn 3y ago
  The AI is adding comments like these: private void makeAllWarningsFatal(Project project) { /** * Makes all warnings fatal for the given project. * * @param project the project for which to make warnings fatal */ What does this achieve? The code already has readable function names. It just creates clutter and makes things worse by forcing people to scroll more by making less code fit in a window. What would be useful is commenting why a setting to make all warnings fatal is a use case. For example, a comment stating that this is used to improve code quality and reduce risk by forcing the developer to fix all warnings which could be hiding errors. This AI is utterly useless at coming up with that.
  - Freedom2 3y ago
    Playing devil's (GPT's?) advocate here - they can be somewhat useful for autogenerated documentation where you don't have access to the code and only some form of browser or other text documentation. Otherwise I agree with you.

hmeh 3y ago

Given that the only comments worth having are typically those that explain why, and an AI would have a hell of a time knowing why code was the way it was, there's very little chance that this does anything but add noise. It's exactly the type of "productivity enhancement" we should expect from AI right now. That is, as long as you measure productivity by lines of code and nothing else that actually matters to the business.

splatzone 3y ago

Can someone familiar with the Spring Boot codebase confirm if these comments are useful or not? At a glance - and I'm not a java developer - a handful of the comments seem to add some specific context that's not obvious from the code alone: https://github.com/spring-projects/spring-boot/pull/39754/commits/d69317431c623f9b9be07771dba8e1d153b1e991#diff-06d556cdacae547b52a06e42d38030533e1cca0fc0b28c529f993be2d737ddaeR144 https://github.com/spring-projects/spring-boot/pull/39754/co...

ptx 3y ago
I'm not familiar with this codebase, but to me it doesn't seem to be adding any context – it's simply describing what the implementation does. In particular, the comment about unnecessaryExclusions is just adding noise since it's an internal implementation detail that doesn't matter to the caller.

samrolken 3y ago

Comments are most helpful when they explain something that is not obvious by just looking at the code. Comments that just explain what the code plainly does don't add much value. This kind of technology can still be useful in other forms as other comments here note, but this kind of auto-commenting, I really don't see it catching on.

stevage 3y ago

There is an awful lot of this: /\** \* Checks if a library is excluded. \* \* @param library the library to check \* @return true if the library is excluded, false otherwise \*/ protected boolean isLibraryExcluded(Library library) { return library.getName().equals("Spring Boot"); } The comment adds nothing - I'm still confused what this one line function is meant to do or what the word "excluded" means in this context. The thought definitely occurs that whatever comment value can be generated automatically could also (and perhaps should instead) be done locally for the viewer, rather than actually being stored in the codebase. Then it doesn't clutter up the codebase, and also can't get out of sync. Or since these comments are really JSDoc style comments, they could be applied just before documentation is generated.

deleted 3y ago

[deleted]

SquareWheel 3y ago

I appreciate the professional exchange between contributor and maintainer. It was contributed with the expectation not that it would be merged, but that it could be adapted into a useful PR if the maintainers were interested. They weren't, so they respectfully parted ways. Skimming through some of the code comments, this tool actually does a pretty good (though not perfect) job of interpreting each function. It would likely need to be limited to only larger functions though, since many of the smaller or boilerplate functions are self-explanatory. And seeing as this is Java, there are a lot of those!

davesque 3y ago

Seems spammy to drop a 100k line PR of just comments, AI generated or otherwise. No maintainer in their right mind would want to review that.

xyst 3y ago

Blatant advertisement for their product. Shameful pull requests like this need to be deleted/purged. Look at this trash: ... /* * Makes all warnings fatal for the given project. * @param project the project for which to make warnings fatal / private void makeAllWarningsFatal(Project project) { project.getExtensions().getByType(AsciidoctorJExtension.class).fatalWarnings("."); } ... Quite possibly the most useless javadoc I have seen. Code itself is self documenting. It's like when the developer gets paid per line of code, this is what is generated.

PaulKeeble 3y ago

I am not impressed. These comments don't add much at all. There are some that superficially look OK and to add some knowledge but in practice are wrong. Then there is stuff like this that doesn't need to be there. /** * Returns the type of the artifact release. * @return the type of the artifact release */ public String getType() { return this.type; } But then you also have the following which looks reasonably promising and does contain useful information that you would only get from the code or the docs. /** * Constructs a new instance of AutoConfigurationMetadata. * * This method retrieves the inputs and sets the file path for the * AutoConfiguration.imports file located in the * META-INF/spring/org.springframework.boot.autoconfigure directory. The path * sensitivity is set to RELATIVE and the property name is set to * "org.springframework.boot.autoconfigure.AutoConfiguration". * * The method also sets a dependency on the processResources task name of the source * set. * * Additionally, it creates a configuration named * "AutoConfigurationPlugin.AUTO_CONFIGURATION_METADATA_CONFIGURATION_NAME" if it does * not already exist in the project's configurations. */ public AutoConfigurationMetadata() { getInputs() .file((Callable<File>) () -> new File(this.sourceSet.getOutput().getResourcesDir(), @@ -68,19 +84,35 @@ public AutoConfigurationMetadata() { .maybeCreate(AutoConfigurationPlugin.AUTO_CONFIGURATION_METADATA_CONFIGURATION_NAME); }*

chuckhend 3y ago

A very kind response by someone from the project team

rsynnott 3y ago
Overly kind, I would say.

Jupe 3y ago

No, no, no! Tell me something useful about the code, or why it is there in the first place... Tell me where it's called from, how often it's called, if the caller can ever send null args, expected execution time, who wrote it, and when, and what else changed when it was first created, whether it is executed under a unit test or functional test. But please do not tell me what it is doing... That's the one piece of information I don't need since I'm already looking at the code.