Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Flirt: The Native Backend

blog.buenzli.dev

Post reply on HN
Print view View original

11–16 of 16 posts

Re: Flirt: The Native Backend

#11

Post by skydhash »

CurleighBraces wrote: post #9
ale wrote: post #7

Earlier quoted context omitted.

How is this different than simply committing between changes? Or even asking the agent to commit changes as it edits files.

Because I don't review everything between changes, there might be 10 small commits I review and state I am happy with, then I might prompt the agent to perform some small refactors after the review, I don't want the diff to show me everything in those 10 commits again in the diff ( like the usual PR would ) I want it only to show me new stuff.

If you’re happy with it, why not squash and merge it?

Re: Flirt: The Native Backend

#12

Post by CurleighBraces »

skydhash wrote: post #11
CurleighBraces wrote: post #9

Earlier quoted context omitted.

Because I don't review everything between changes, there might be 10 small commits I review and state I am happy with, then I might prompt the agent to perform some small refactors after the review, I don't want the diff to show me everything in those 10 commits again in the diff ( like the usual PR would ) I want it only to show me new stuff.

If you’re happy with it, why not squash and merge it?

That's now how it works though is it? ignoring agents

Junior dev working on feature X

Junior dev makes a lot of commits and creates PR

Senior dev reviews code, spots problems a,b,c but has reviewed all the code at this point. Feature X isn't complete it can't be merged

Junior dev fixed a,b,c now 90% of the already reviewed code might not have changed but the PR doesn't show that.

Now replace junior dev with agent.

Re: Flirt: The Native Backend

#13

Post by saghm »

exceptione wrote: post #6

> Imagine a commit in a patch series changes 100 lines. 99 of the lines are good, but the first round of review suggests to add a missing semicolon on one of the lines. The author does that by amending the existing commit, to avoid an unseemly "add missing semicolon" commit ending up in the project history. After a force-push (or new patch series submission), reviewers are generally presented with the full 100-line d…

Yeah, I will happily make small commits like this and then squash all of them before merging. There's been a built-in option for this on Github that's been around for close to a decade at this point and is easy to make the default. I don't feel like this is a real problem.

Re: Flirt: The Native Backend

#14

Post by WorldMaker »

josephg wrote: post #8
exceptione wrote: post #6

> Imagine a commit in a patch series changes 100 lines. 99 of the lines are good, but the first round of review suggests to add a missing semicolon on one of the lines. The author does that by amending the existing commit, to avoid an unseemly "add missing semicolon" commit ending up in the project history. After a force-push (or new patch series submission), reviewers are generally presented with the full 100-line d…

Urgh. This problem with git is so unforced. Essentially, people want commits to mean 2 different things at the same time: 1. Store the uncurated history of how the project was developed. 1 commit is created whenever a dev types 'git commit'. 2. Be a linear list of PRs / changes / whatever to a project. Each PR is created by squishing many commits together. It is associated with a conversation around code review, a ti…

> The answer seems pretty obvious. Git just needs 2 separate mechanisms for these 2 different ideas. The obvious approach would be to leave commits as being atomic units of code change, and invent a new concept in git which represents a feature / feature branch. It would essentially just be a metadata object naming a set of commits, and it would have links to an issue tracker, CI results and code reviews.

git already has a tool for this called a "merge commit". A merge commit is mostly just a metadata object pointing to multiple previous commits. Sure, this forms a DAG rather than a line, but when all you want to see is a line just walk the graph in a specific pattern. In git that graph walking pattern is usually called `--first-parent`.

Mostly we just need UIs that stop trying to draw a "subway diagram" at all times and just does a `--first-parent` overview. It's strange to me how much work is being put into squashes and rebases and more just to avoid complicated "subway diagrams" when that often would be just fine with merge commits and adding `--first-parent` to your git log defaults and/or pick a UI tool that does that for you.

Re: Flirt: The Native Backend

#15

Post by skydhash »

CurleighBraces wrote: post #12
skydhash wrote: post #11

Earlier quoted context omitted.

If you’re happy with it, why not squash and merge it?

That's now how it works though is it? ignoring agents Junior dev working on feature X Junior dev makes a lot of commits and creates PR Senior dev reviews code, spots problems a,b,c but has reviewed all the code at this point. Feature X isn't complete it can't be merged Junior dev fixed a,b,c now 90% of the already reviewed code might not have changed but the PR doesn't show that. Now replace junior dev with agent.

That would be pair programming with extra steps if you're just reviewing updates and not the whole patch that is going to be merged (and which is stamped with your approval).

Re: Flirt: The Native Backend

#16

Post by CurleighBraces »

skydhash wrote: post #15
CurleighBraces wrote: post #12

Earlier quoted context omitted.

That's now how it works though is it? ignoring agents Junior dev working on feature X Junior dev makes a lot of commits and creates PR Senior dev reviews code, spots problems a,b,c but has reviewed all the code at this point. Feature X isn't complete it can't be merged Junior dev fixed a,b,c now 90% of the already reviewed code might not have changed but the PR doesn't show that. Now replace junior dev with agent.

That would be pair programming with extra steps if you're just reviewing updates and not the whole patch that is going to be merged (and which is stamped with your approval).

Not sure if it's just my age but pair programming seems to have a different meaning these days.

It used to mean that two developers would literally sit next to each other, one would type and one would review as the person typed, then they would repeat.

I guess in the world of remote working it's just not practical any more.

I certainly wouldn't call, one person reviewing another persons code during the merge process pair programming at all.

Post reply on HN