search

watermelontools / watermelon

🍉 Open-Source Copilot For Code Review
https://app.watermelontools.com
Apache License 2.0
143 stars 12 forks source link

Not making comments #415

Open Snazzie opened 6 months ago

Snazzie commented 6 months ago

The bot isnt making comments, it only applies "Taking a deeper dive" and "Safe to merge"

i know for a fact that my code is abysmal.

https://github.com/Vital-Utilities/Vital-Utilities/pull/119

baristaGeek commented 6 months ago

Hey @Snazzie thanks for your feedback!

Is it abysmal because of the type annotation warnings? Or because of what else?

Also, right now we're only detecting console logs and their equivalents in other languages (I saw that in your PR we're incorrectly detecting lines of the yml file as console logs) . This is the PR just in case you want to take a look and give us some feedback https://github.com/watermelontools/watermelon/pull/413 What else would you like us to detect? Leftover comments? PII Data? Something else?

Finally regarding the pre-review label. Do you mean our labeling criteria should be more strict? (More PRs getting "Don't Merge" and "Take a deeper dive" instead of "Safe to Merge"?)

rogerluan commented 6 months ago

I'm here to +1…

So far watermelon run in several PRs of mine, in Ruby and Swift codebases, and it never commented on actual lines of code, just some summaries that are not very useful so far 😬

Examples:

Any clue why is that? My theory was because it doesn't yet support Ruby or Swift, but I can't really tell, since it's not clear anywhere what are the languages it officially supports 😅 so that's another feedback to you guys 🙇

After reading the comment above "we're only detecting console logs", does this mean it will only comment whether it has console logs or not? 🤔 couldn't this be done using Danger? There's no AI in that 😅

baristaGeek commented 6 months ago

Hey @rogerluan, thanks for your input!

Just to clarify, our current focus is on pinpointing console logs within codebases. However, we're actively working on expanding our capabilities to include PII data detection.

Is there anything else you'd like us to keep an eye out for? Are you thinking about things like leftover comments, or perhaps detecting comments that don’t really add value (redundant)? Any other suggestions? We're all ears!

As for the summary, it's primarily designed for companies rather than individual projects. It's especially handy when your code context is spread across platforms like Slack, Jira, Notion, etc. Do you have any thoughts on how we could tailor it to be more beneficial for open-source projects?

rogerluan commented 6 months ago

Do you have any thoughts on how we could tailor it to be more beneficial for open-source projects?

tbh a summary wouldn't be useful as an after-thought (i.e. in the form of a comment posted by a bot in the PR after it is open). Instead, if the bot could write a draft for the PR description while the author is still in the process of opening the PR (e.g. a browser extension?), then this would be super valuable and save developers' time.

Is there anything else you'd like us to keep an eye out for?

My tip is for you to aim solving problems that can only be solved by LLM, if you're really working with an LLM. All examples listed (detecting - and kinda interpreting - console logs, accidental PII, leftover comments) can be more or less be identified/fixed using procedural programming, thus not requiring LLM. Of course, the results with LLM can be slightly better, but it's probably not an edge.

My suggestion here is for you to seek making the bot's review as good as a human's. There are lots of static code review services out there that do somewhat advanced analysis e.g. cyclomatic complexity, common code smells, etc, so if you want to stand out, you'd have to offer what those can't offer without an LLM. Things that require context, interpretation, viewing the bigger picture, etc. I can't pinpoint specific examples, but hopefully these will spark ideas for you (maybe with the help of GPT 😜)