PR title and description gets completely wiped on update or merge

[awelm]

Hi!

I love this tool and I really want to keep using it. But i can't really use it on the job if it always overwrites my PR description because that field includes a lot of context, test plan, etc. It would also be nice if PR title changes via the UI were preserved too.

Thanks, Akila

[ejoffe]

Hi Akila,

I personally add all the information including test plan, etc into the commit message. The nice thing about that is the information is stored in git history as well, and not gone once the pr is closed.

[ppwwyyxx]

@ejoffe though it is a valid way to put information in commit message, and has the benefits you said, an obvious significant downside is that editing commit message has very poor experience compared to github -- github supports markdown, supports uploading photos, and gives you a markdown preview of what's written.

I'd like to see the PR title & body preserved. And even better, it would be great if spr only updates the parts of the PR body that needs to be updated (i.e. the stack). Another similar tool (ghstack) provides this option so that title & body are not wiped: https://github.com/ezyang/ghstack/blob/16fd53fe7f59adb77e098f0ea7f0453bfce498b6/ghstack/submit.py#L370-L371

[ejoffe]

We can add a configuration flag so that once a PR is created, the body of the PR will no longer be updated on subsequent spr updates.

[ejoffe]

PreserveTitleAndBody config added. With this enabled the stack info is not printed, having that is beyond the scope of this change. Can open a separate issue to track that feature.

[dko-slapdash]

@ejoffe JFY, I stepped on it within the first 5 minutes of evaluation of the tool, and my first reaction was "it's a bug, it destroyed the text and screenshots which I just spent 10 minutes earlier on!". The next reaction was a give-up and thinking about using some other too. (I'm ex-FB BTW.)

I believe that having preserveTitleAndBody=false by default is super-dangerous. No software should ever delete the user's generated texts without a warning or without an explicitly provided option.

My expectation from the default behavior was that if I update a PR description in GitHub UI, next time I run spr updated, it will pull that updated description from there and update it to my local commit. (Currently, the tool does the opposite.) And actually, even if it were to update the local commit from the remote text, it should've still asked my confirmation in case I earlier changed my local commit message (to not let me lose even that text). It can be easily memoized right in the commit, by adding something like commit-texts-hash:xyz to the commit once it's pushed to GitHub with spr update.

As a summary, here are examples of best-effort behavior (in my opinion):

1. I commit and run spr update. Then I go to the PR at github.com (and I ALWAYS do it like 90% of other people, to ADD SCREENSHOTS by drag-dropping them, to take a final look at the message, and also to take a final look at the code which I submitted). I update PR texts to fix typos (of course; I do it in 95% of cases), and next time I run spr update, these changes are auto-applied to my local commits, it never loses my texts.
2. I commit and run spr update. Then I modify the local commit message and run spr update. (I did no change title/description at GitHub PR) and run spr update. It then should silently update the title/text at github.com PR (it's safe).
3. I commit and run spr update, then I modify title/description at github.com AND, after some time, accidentally modify title/text in my local commit. In this case, the tool detects that I modified the text locally (in comparison to what is in commit-texts-hash header in the commit message) AND remotely, and it should now ask me explicitly, which message do I want to update at both places (showing me both messages on the screen, and maybe even showing the diff of the messages). I can decide what to do: use the remote message or use the local message.

Maybe commit title and description should even be tracked independently. I imagine a case when I update the comment title locally and then spr update, and I expect this change to propagate to github.com PR (because I haven't changed anything in GitHub PR title, so it's safe to update, it wouldn't lose any user generated text), but at the same time, I don't want to erase-override the description in the PR. And vice versa. The general idea is that we detect the change not across "local and remote", but separately "there was a change in a local field" and "there was a change in the remote field", and then act accordingly, to never lose that bit of the change.

[ejoffe]

@dko-slapdash - Hi Dimitri, sorry that you lost your pr details. This tool is built for my use case in which the commit message is the source of truth, I NEVER muck with the github ui, to me that's a complete waste of time. I want the source of truth to remain in the commit message and not be lost once the pull request is merged. I get that you ALWAYS do this, but I don't think you should generalize for 90% of other people, so far you are the only one that feels so strongly about this, with probably well over thousands of users of this tool. Having said that, I do think your point is valid, being an open source tool, there has been many contributions from the community. If this is something that you feel strongly about and want to add to spr, I'll highly appreciate it as will the spr community. You can also look for other similar tools that might fit your use case better.

[dko-slapdash]

@ejoffe Curious how do you upload screenshots then? Or - how do you make sure Markdown typography is correct? (I'm generalizing to 90%, because I have reasons for it: in both FB an all other companies I was a part of, screenshots were added to the PRs very frequently, like in 2 PRs out of 5, and the easiest way to add a screenshot is Shift+Cmd+5 and then paste it to GitHub UI; the same applies to videos).

Regarding the source of truth being local - nothing contradicts to it if spr update could sync the changes from GitHub PR back to the local commit iff the local commit message is unchanged (and we can verify that it's really unchanged). This way, the user would be able to apply changes to both local and remote ends without losing a bit.

[dko-slapdash]

BTW, this is also how it worked in FB: you could do whatever you want in Phabricator UI, and it never erased anything anywhere.

[lukas-mertens]

@ejoffe I just found this issue. Even though I don't feel as strongly as @dko-slapdash about the default, it would indeed be nice if spr would only replace what it created before. That is pretty easy to do actually and done like that by other tools as well. GitHub supports html comments, so you can just do the following:

<!-- SPR Description Start: Don't edit anything inside or it will be lost -->
All the spr stuff here
<!-- SPR Description End: Don't edit anything inside or it will be lost -->

And then you just do a replace inside those two hardcoded strings. That's how e.g. coderabbit.ai does it and it works pretty well.