| opendevreview | Michal Nasiadka proposed openstack/project-config master: propose-updates: Add pcu target https://review.opendev.org/c/openstack/project-config/+/978566 | 12:39 |
|---|---|---|
| opendevreview | Michal Nasiadka proposed openstack/project-config master: propose-updates: Add pcu target https://review.opendev.org/c/openstack/project-config/+/978566 | 12:58 |
| opendevreview | Michal Nasiadka proposed openstack/project-config master: propose-updates: Add pcu target https://review.opendev.org/c/openstack/project-config/+/978566 | 13:06 |
| opendevreview | Merged openstack/project-config master: Stop Venus gating https://review.opendev.org/c/openstack/project-config/+/981944 | 13:23 |
| clarkb | sean-k-mooney: Gerrit 3.13 (we just upgraded to 3.12 so aren't affected yet) adds a "Create AI Review Prompt" button to change pages. YOu can see it here: https://gerrit-review.googlesource.com/c/git-repohooks/+/572507 You can click on that without logging in and it pops up the little window with details including a suggested prompt. I don't think you need to do anything | 17:35 |
| clarkb | about this and this is discussion upstream on the mailing list about making this more opt in. But I thought you might be interested to compare and contrast with the setup you've been using so figured I'd mention it | 17:35 |
| sean-k-mooney | i noticed that 3.12 seams ot have chagned a bunch fo fonts and minor ui things | 17:40 |
| sean-k-mooney | active si now? yellow | 17:40 |
| sean-k-mooney | i think it was green before | 17:40 |
| sean-k-mooney | we also now seam to have the check ui | 17:41 |
| sean-k-mooney | instead fo the findign one | 17:41 |
| sean-k-mooney | oh that on that thidparty one | 17:41 |
| sean-k-mooney | on our one we are still usign the zuul summary plugin | 17:41 |
| sean-k-mooney | althoguh active is now yellow on that too | 17:41 |
| sean-k-mooney | clarkb: so this just provide a prompt you can run locally | 17:42 |
| sean-k-mooney | is that somethign you can configure diffeently per project in gerrit | 17:43 |
| sean-k-mooney | the current promps are not greate and proably not somehtign we woudl want to enable | 17:45 |
| sean-k-mooney | the main thing i will say about the review promet is its very short and does nto include any project convetions or guidence so its likely not going to provide good output as a result | 17:46 |
| fungi | maybe it's tunable/customizable via acls | 17:50 |
| fungi | or could be made so in a future version if now | 17:50 |
| fungi | if not | 17:50 |
| sean-k-mooney | what i think i would like us to do inthe next year or two is crate a comuity mainted agent-tools repo or simlar and provide a review skill (liek the one inmy repo) taht could be used if folks want to do "review the openstack way" locally with an llm | 17:52 |
| sean-k-mooney | somethign that woudl know where to look in openstack code repos for review guedance, docs and convetniosn to apply | 17:52 |
| sean-k-mooney | but i think we are going to need to continue to build expirce with usign tools in this way before we will have somethign to share with contibutors in general | 17:53 |
| clarkb | sean-k-mooney: yes lots of minor UI tweaks but I don't thik anything drastically changed in 3.12. We also don't have the check UI. I linked to the upstream gerrit so you could see the prompt stuff which is not yet in our gerrit and they use checks | 17:54 |
| clarkb | and no I don't think much of that is tunable. The new discussion on their mailing list has been prompted by a deployment that does not allow any ai at all and wants to turn it off | 17:54 |
| sean-k-mooney | ya the checks ui mirror githubs checks ui which kind fo sucks but it is what peopel are familar with | 17:55 |
| sean-k-mooney | clarkb: ya for now i would proably default to off as well | 17:55 |
| clarkb | and I don't know how much of that will end up being configurable before we upgrade to 3.13. Well you can't even default it to off | 17:55 |
| clarkb | we don't have it because 3.12 doesn't support it | 17:55 |
| sean-k-mooney | ack is it at least a plugin or did they put it in the core | 17:56 |
| clarkb | I think it is in core. But that is ag ood question. If they did it as a plugin then ya we could just avoid installing that. Let me do more digging | 17:56 |
| sean-k-mooney | ack. i know they had a few attepts to add llm reivews via plugins | 17:56 |
| clarkb | it is in core https://gerrit.googlesource.com/gerrit/+/refs/tags/v3.13.5/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts#567 but there is that check for aiPromptModal so maybe you need a plugin to define that? I'm still digging | 17:58 |
| sean-k-mooney | https://paste.opendev.org/show/bCNmVaMiG02cMMq9OFoy/ is there review promt in its entirity - the inline patch diff | 17:59 |
| clarkb | https://gerrit.googlesource.com/gerrit/+/refs/tags/v3.13.5/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/prompts.ts they seem to have hardcoded the template into the ts soruce even | 18:00 |
| clarkb | so ya I dont' think this is currently configurable at all | 18:01 |
| sean-k-mooney | so 36 lines, claudes building review skill is ~ 100 lines https://github.com/anthropics/claude-code/blob/main/plugins/code-review/commands/code-review.md and mine is curently about 800 althoug i can shorten that a bit and will in the future | 18:01 |
| clarkb | like you can't even change the prompt text | 18:01 |
| sean-k-mooney | clarkb: it also does nto ahve a id... so youc ant easilly hide it with javascript or css | 18:01 |
| clarkb | https://gerrit.googlesource.com/gerrit/+/refs/tags/v3.13.5/polygerrit-ui/app/elements/change/gr-ai-prompt-dialog/gr-ai-prompt-dialog.ts#33 I was hoping they would load it from config or check config but no its just hardcoded I think | 18:01 |
| clarkb | so we'll see where this discussion taht was started today goes | 18:02 |
| clarkb | https://gerrit-review.googlesource.com/c/gerrit/+/479041 this is the change that dded it which does say there is a flag you can set. But I don't see that in the code. I'm digging more | 18:03 |
| sean-k-mooney | GET_AI_PROMPT experimntal flag | 18:04 |
| sean-k-mooney | but that https://gerrit-review.googlesource.com/c/gerrit/+/479041/5/polygerrit-ui/app/services/flags/flags.ts | 18:04 |
| sean-k-mooney | but i asume that will go away | 18:04 |
| clarkb | it was a two month long experiment: they removed the flag here: https://gerrit-review.googlesource.com/c/gerrit/+/510984/5/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts | 18:05 |
| sean-k-mooney | GET_AI_FIX = 'UiFeature__get_ai_fix', | 18:05 |
| sean-k-mooney | taht could also be problematic | 18:05 |
| clarkb | so that experiment didn't even go out for one release | 18:05 |
| clarkb | I think we can make an argument that experiments shouldn't be that short and not even last for a single release | 18:05 |
| clarkb | I'll work on a response to the mailing list | 18:05 |
| sean-k-mooney | ack, i think if this is to be a thing it need to be opt in and the prompt need to be configureabl at least at the site level but idealy at the project level simialr to the acls | 18:06 |
| clarkb | yes I think project level is important beacuse each project can have different appraoches to the use of AI and also the way they want to prompt llms for reviews | 18:07 |
| fungi | experimenting on deployments that continuously deploy from the bleeding edge of the development branch, essentially | 18:07 |
| fungi | i agree wrt project level control and tuning as previously mentioned, since gerrit is often used in multi-tenant environments | 18:08 |
| sean-k-mooney | ya kata and openstack ro zuul may have very diffent approches | 18:09 |
| sean-k-mooney | personally i dont think diff based reivew si good ro useful | 18:09 |
| sean-k-mooney | taht why i build my stuff to work on teh git repo level | 18:10 |
| sean-k-mooney | so it can fully exproe the git tree for context | 18:10 |
| sean-k-mooney | having a prompt that inlines the diff that you copy past is very chatgpt 2023 vibes | 18:11 |
| clarkb | https://groups.google.com/g/repo-discuss/c/duY8pKj3qBg/m/wIp2BTNmBwAJ this is the response I sent | 18:15 |
| sean-k-mooney | i get the desire to supprot this since github/gitlab ectra have integration for diffent review services | 18:18 |
| sean-k-mooney | but they all make it confiugable per project/namespace | 18:19 |
| sean-k-mooney | so not supproting that seems like a big miss | 18:19 |
| clarkb | yes I think supporting it makes total sense. but when you add features like this you add documentation, you add the ability to enable/disable it, and you make the templates configurable. Gerrit already does this for 99% of the stuff they add | 18:19 |
| sean-k-mooney | espiclaly given hwo well gerrit genrally suprpot that | 18:19 |
| clarkb | it feels like they rushed this and didn't worry about the larger gerrit community (when designing it and when ensuring it could be used by anyone else) | 18:19 |
| fungi | right, seems like an outlier to me | 18:19 |
| clarkb | I'm hoping my email is sort of a clear "this is the sort of thing that the rest of us expect" outline for new features. I have nothing against the feature. Its just surprising how underbaked it appears | 18:20 |
| fungi | the cynic in me thinks like smells like something that baked for a while inside a company and then got copied to the public version of the project after having been used by a monocultural organization without complaint | 18:21 |
| fungi | so per-project configurability wasn't even on their minds | 18:21 |
| sean-k-mooney | maybe but the prompts seem rather lack luster | 18:22 |
| clarkb | which is a non issue if they allow them to be configurable | 18:22 |
| clarkb | then we can all have our own lackluster prompts :) | 18:22 |
| JayF | The built-in prompts ... yeah what sean-k-mooney just said | 18:26 |
| JayF | those prompts are surprisingly bad to not be customizable lol | 18:26 |
| sean-k-mooney | the main proble is whil it ask "**Consistency:** Are there any inconsistencies with existing code or design patterns?" or "* **Style:** Does the code adhere to the project's coding style guidelines? Is it readable and maintainable?" | 18:29 |
| sean-k-mooney | since it intende that you inlien the patch and then past this into an agent | 18:30 |
| sean-k-mooney | unless you first checkout the repo with the chagnes applied | 18:30 |
| sean-k-mooney | it cant asnswer that form the diff alone | 18:30 |
| sean-k-mooney | so if you jsut past the promt it provide into say chatgpt ro calude.ai's web interface it will not be able to revew properly | 18:31 |
| clarkb | right the risk is that it will be treated as an attractive nuisance producing poorly conceived -1 reviews from people who don't know better | 18:34 |
| clarkb | I think if we upgraded to 3.13 today and got that feature then the core reviewers could learn to just ignore it. The problem is people who are less familiar with Gerrit and use it infrequently and see it as an easy way t obe helpful. Not the end of the world, but definitely a likely source of issues | 18:34 |
| fungi | i can be helpful by clicking this button that it looks like nobody else has clicked yet | 18:35 |
| sean-k-mooney | well at least its opt in | 18:36 |
| clarkb | sean-k-mooney: no it isn't | 18:36 |
| clarkb | thats the problem | 18:36 |
| clarkb | it isn't even opt out | 18:36 |
| clarkb | it is just there with its hardcoded templates as far as I can tell | 18:36 |
| sean-k-mooney | well what i mean is its not auto runnitn the prompt and adding a comment with that promt | 18:36 |
| fungi | oh look! halfbakedai says god uses three-space indents | 18:36 |
| clarkb | sean-k-mooney: oh right a human does need to take action to post the resulting comments | 18:36 |
| clarkb | yes, that does mitigate a lot of the problems | 18:37 |
| sean-k-mooney | well that and its not makeint it even more proment | 18:37 |
| sean-k-mooney | like advertising it in the email notifications ectra | 18:37 |
| sean-k-mooney | jira keesp telling me `Hint: You can mention someone in a work item description or comment by typing "@" in front of their username` | 18:38 |
| sean-k-mooney | and im like i know i didnt use the @ becuase i dont want to ping them | 18:38 |
| fungi | it's clippy all over again | 18:39 |
| clarkb | oh I hate when software nags you to use the new feature you are intentiaonlly avopiding | 18:39 |
| clarkb | slack does this now | 18:39 |
| fungi | it looks like you're trying to file a bug... | 18:39 |
| clarkb | I never liked slack anyway but now I'm like wow | 18:39 |
| sean-k-mooney | fungi: maybe clippy was just ahead of its time | 18:40 |
| fungi | microsoft bob taught clippy everything it knows | 18:40 |
| JayF | Honestly the shape of this feature feels like someone half-implementing something upstream | 18:42 |
| JayF | so they can do something more invasive downstream with less annoying rebases | 18:42 |
| fungi | yes, it helps to remember that gerrit is used internally by a lot of teams in google | 18:43 |
| clarkb | in this case I think the upstream is the downstream (google seems to have dev'd it and I think they run the web ui as is) | 18:43 |
| clarkb | they replace a lot of stuff using dependency injections but I think the UI is used as is at least for the public facing projects like chromium | 18:43 |
| JayF | fungi: I know, that's sorta what I was thinking | 18:43 |
| JayF | also with Sean and I being like "what bad prompts", I don't think either of us use much gemini | 18:43 |
| JayF | so it may just be optimized for googly people | 18:44 |
| fungi | that's what i was getting at earlier with the feature seeming like it came from earlier testing in a monoculture | 18:44 |
| sean-k-mooney | well gemini prefers both claudes adn my review prompt over gerrit | 18:44 |
| sean-k-mooney | JayF: i use it for email spellcheck adn thats about it | 18:44 |
| sean-k-mooney | i ocationally try it for other thigns but we only have access to it via google workspace so no coding agants | 18:45 |
| fungi | the prompt text may be a baked-in placeholder/example that they didn't bother to add tuning for because they patch it themselves in their own deployment | 18:45 |
| clarkb | oh ya that could be | 18:45 |
| clarkb | I bet that is what is happening | 18:45 |
| clarkb | and maybe my read that you can't configure the templates like email is wrong too | 18:45 |
| sean-k-mooney | they appreaed to just be typesciprt "contatns" | 18:46 |
| sean-k-mooney | but maybe there is a way to orverried in a later patch | 18:46 |
| JayF | Rackspace may have gotten IRonic to add a rescue interface a good release before the overall design was ready | 18:46 |
| sean-k-mooney | those could eb the defaults | 18:46 |
| JayF | sometime back in 2015 :D | 18:46 |
| JayF | for this basic reason | 18:46 |
| clarkb | sean-k-mooney: ya my git grep for `IMPROVE_COMMIT_MESSAGE` doesn't show it popping up anywhere that looks like a use this as a default. It just uses it. But bazel and ts and all that are not tools I use every day so there may be some magic involved that enables this | 18:48 |
| clarkb | JayF: wait until you learn about the glance image import tasks api | 18:48 |
| clarkb | something that was added and quite literally never finished. but we're still using it every day in opendev | 18:49 |
| fungi | everyone on the vmt is (i think) painfully aware of the glance tasks api^H^H^Hfiasco | 18:49 |
| sean-k-mooney | clarkb: well its typescript/javascript so you can alwasy edit it in the console fi you want i guess :P | 18:51 |
| sean-k-mooney | we coudl add a tiny plugin to sed them but ya lets see if they respodn to your mail | 18:51 |
| sean-k-mooney | what really gets me is there prompt dont even include a refence to the gerrit chagne under review | 18:53 |
| sean-k-mooney | if you just give an agent that its able to fetch the review contxt form the rest api | 18:53 |
| JayF | this is why I assumed it's an empty vessel that a downstream poured a feature into | 18:53 |
| JayF | because otherwise it's a dumb feature, frankly | 18:53 |
| JayF | maybe someone got "AI synergies with open source gerrit" on their review and got an extra .5% raise for it | 18:54 |
| fungi | clearly i'm in the wrong line of work! ;) | 19:00 |
| fungi | granted, they *do* have to code in java, that merits hazard pay | 19:01 |
| sean-k-mooney | apprely "new" java is less terrible but ya i kind of treeat java, javascript and perl as write only lanagues | 19:04 |
| sean-k-mooney | becuase ocne anything is written in them you never want to need to read it again | 19:04 |
| clarkb | imo java is more than fine | 19:04 |
| clarkb | the thing that makes java annoying is everyoen thinks you need a giant IDE to write java. You don't I'm in the vim, jdb, and bazelisk gang here when doing gerritstuff | 19:05 |
| sean-k-mooney | the bit that use to annoy me was the maven vs gradel switch and ya eclipse but netbeans and intellj were fine | 19:06 |
| fungi | any time i touch a large java project, it's a spaghetti mess of cross-referential files with one class each, overloaded by type | 19:06 |
| fungi | hard to keep all the context in my head | 19:06 |
| clarkb | yes the layout of java projects hasn't chagned much. New java is more about adding other quality of life improvements liek anonymous functiosn and filters and such | 19:06 |
| clarkb | so the code you write doesn't have to be quite so verbose. but you still end up with a class per file typically | 19:07 |
| clarkb | also they made massive improvements to the runtime and things like garbage collection | 19:08 |
| fungi | i may be part of the problem. i'm more comfortable in scripting languages where i can typically get away with loading a single file at a time into my editor | 19:08 |
| clarkb | big stop the world collections are largely a thing of the past | 19:08 |
| sean-k-mooney | clarkb: so the most recent expicnce i had with it was minecraft moding about 4--5 years ago and ya the changes to the garbage collector broke a lot of things but also make the runtime much leaner | 19:09 |
| sean-k-mooney | a lot of that was driven by scala and kotlin devleopemt too | 19:09 |
| sean-k-mooney | its become a "modern" language with corrtiens and lamdas you know all the best 70s tech | 19:10 |
| fungi | modern like lisp | 19:11 |
| sean-k-mooney | fungi: for what its worth for the first 4-5 years of contibuting to openstack i basiclly jsut used nano | 19:11 |
| fungi | i still do all my coding in vi/vim/neovim | 19:11 |
| sean-k-mooney | unless i nded to debug somethign then i used intelej/pycharm and went back to print debuging when evently woudl break that | 19:11 |
| sean-k-mooney | so its one of the thing im both excited and dreading with the eventlet remvoeal | 19:12 |
| sean-k-mooney | inteory a debugger is not a thing we can use again | 19:12 |
| sean-k-mooney | i just knwo if i need ot reach for ti im debuging somethign annorying | 19:12 |
| fungi | because eventlet breaks pdb? | 19:12 |
| sean-k-mooney | eventlet breaks breakpoint | 19:13 |
| fungi | yuck | 19:13 |
| sean-k-mooney | so ya you cant singel step debug | 19:13 |
| sean-k-mooney | you could set a break poitn and then run till you hit it but condtional breakporint used to not work reliable either | 19:13 |
| fungi | i wonder how well that works with he more recent threading models in cpython | 19:13 |
| sean-k-mooney | that used to work jsut fine. i used to jsut comment out eventlet monkey patching if i neede a debuger | 19:14 |
| fungi | like i have no idea if breakpoints even work reliably with asyncio | 19:14 |
| sean-k-mooney | the proble was you woudl break other thing as a result fo that | 19:14 |
| sean-k-mooney | fungi: that i dont knwo i have not tired it but most openstack proejct are not using asyncio currently | 19:14 |
| sean-k-mooney | i woudl have ot assume that eaiser to supprot since its in the standar lib | 19:15 |
| fungi | yeah, i just meant more generally, not in an openstack context | 19:15 |
| fungi | i don't do a lot of thread-aware programming | 19:15 |
| sean-k-mooney | like i know gdb can handel c++ (llvms) corutines fine | 19:15 |
| fungi | to be honest, multithreading still confuses me at times. i have to think of it in terms of distributed multiprocessing and message-passing routines i'm familiar with (pvm/mpi/mpich) | 19:16 |
| sean-k-mooney | to be fair if we worte our concurancy in those terms it woudl likely be more robust in general | 19:17 |
| fungi | more robust but probably spinlocking on semaphores and mutexes | 19:18 |
| sean-k-mooney | well we have those anyway | 19:18 |
| sean-k-mooney | the way nova used evnetlet they were basicly treated as threads. so we have lot of of locks around in memory data sturctes | 19:19 |
| sean-k-mooney | baiclly we had to treat all shared sate as if it was in a pthread anyway before we started on the eventlet removal | 19:19 |
| fungi | discussions about newer concurrency models seem to be in terms of "function colo(u)ring" which i still don't grok | 19:20 |
| sean-k-mooney | becailly because every io like ever log call could context switch | 19:20 |
| sean-k-mooney | fungi: well that bacilly because of python | 19:20 |
| fungi | i guess log calls aren't just dumping into a queye for a separate stream writer | 19:21 |
| sean-k-mooney | i.e. in python an async function can call a normal funciton but not the other way around | 19:21 |
| sean-k-mooney | so in python at least if you provide a async def api your forcing all the caller of that api into that world and effectivly you have 2 types or colors of fucntions | 19:22 |
| sean-k-mooney | the async world and everythign else | 19:22 |
| sean-k-mooney | that not hwo many other languages work | 19:22 |
| fungi | ah okay, so very python-specific terminology then | 19:23 |
| sean-k-mooney | python was the first popular lange to have that device adn a few other inheritied after | 19:23 |
| sean-k-mooney | but other learned form the mistake and avoid that expliclty | 19:23 |
| fungi | i mostly see it brought up in contrast to concepts like golang's "goroutines" (which is their coroutine implementation i guess) | 19:24 |
| sean-k-mooney | ya so in go you write a function and then the caller etier calls it direction or put the "go" statemet infront of it and makes it async with a future returned | 19:24 |
| sean-k-mooney | so the lanauge runtime allwos the caller to decied if it shoudl be a blockign call or not | 19:25 |
| sean-k-mooney | there are other aspect to that | 19:25 |
| fungi | which i suppose could be more error-prone as a result | 19:25 |
| sean-k-mooney | but that kind of the main one a fucntion is not sync or async based on itd defintion its a chosice of the caller | 19:25 |
| sean-k-mooney | well if you as the lib writer had one assumtion and the caller had anohter it could lead to odd behvior | 19:26 |
| sean-k-mooney | the asusmetion with go is its cheap to span many go rutiones and they will comunccte with channels | 19:26 |
| sean-k-mooney | which are just queue | 19:27 |
| sean-k-mooney | so if your fucntion is actully expensive and you create 1000 isntnace of it :) | 19:27 |
| opendevreview | Merged openstack/project-config master: Bump jeepyb gerrit jobs up to gerrit 3.12 https://review.opendev.org/c/openstack/project-config/+/983475 | 19:33 |
| *** bauzas1 is now known as bauzas | 22:40 | |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!