Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo: bump scm-record to 0.4.0 #4639

Merged
merged 1 commit into from
Oct 15, 2024
Merged

Conversation

farnoy
Copy link
Contributor

@farnoy farnoy commented Oct 14, 2024

Fixes #4001
Fixes #4635

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

Copy link

google-cla bot commented Oct 14, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@arxanas arxanas requested review from martinvonz and arxanas October 14, 2024 08:45
Copy link
Contributor

@arxanas arxanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM! @martinvonz do we have a procedure for remarking on the upgrade in the changelog? (Is it worth remarking? Do we list individual bugs that were fixed, or just say "scm-record was upgraded to v0.4.0" and link to its own release notes?)

@martinvonz
Copy link
Member

Thanks, LGTM! @martinvonz do we have a procedure for remarking on the upgrade in the changelog? (Is it worth remarking? Do we list individual bugs that were fixed, or just say "scm-record was upgraded to v0.4.0" and link to its own release notes?)

I don't think we have a procedure for it. Since scm-record is so closely integrated that most users seem to think it's simply part of jj, I think it makes sense to at least mention the most important changes in our own changelog. What do you think?

@arxanas
Copy link
Contributor

arxanas commented Oct 14, 2024

Sounds good to me. I think this would also resolve #4001, and maybe #4502 (we now have a help dialog that just says to click the menu bar items).

@ilyagr
Copy link
Contributor

ilyagr commented Oct 14, 2024

It would be great to have a brief summary of new builtin diff editor features inside the normal Changelog "New Features" and "Bugs Fixed" section. If there are many, you can add "For more details, see <link to scm-record release notes>".

But if that feels like a chore, you could just add a line to one of those sections "The built-in diff editor has been updated to scm-record 0.4.0", followed by a link to the release notes if that exists.

Note also that we can update the Changelog further in a separate PR.

@farnoy
Copy link
Contributor Author

farnoy commented Oct 14, 2024

FWIW, the original issue that motivated this PR isn't mentioned in scm-record's changelog, so I'm not sure how detailed jj's changelog could be.

IMO, just mentioning a dependency update isn't that useful, unless there's a major change within.

@arxanas
Copy link
Contributor

arxanas commented Oct 14, 2024

@farnoy I believe it's discussed in this entry:

(#37): Fixed redraw issues when rendering tabs and other non-printing characters.

since the carriage return is not a printing character. But it's apparent that it's not obvious what to look for. In addition to pointing out the dependency upgrade, we could list each of the individual issues that are resolved in the jj repo in the changelog. I was writing "via X upgrade", e.g.

git-branchless now supports index version 4 (via libgit2 upgrade).

in the git-branchless release notes to point out when issues are resolved via dependency upgrade instead of directly.

@ilyagr
Copy link
Contributor

ilyagr commented Oct 14, 2024

Just try to describe what you think is important about the update, to the degree where it's not much trouble for you. It'll probably be fine, and we can edit it later or suggest fixes here if necessary.

@ilyagr
Copy link
Contributor

ilyagr commented Oct 14, 2024

Or, if it is a lot of trouble, I'm personally ok with you merging without a changelog. Just ping me and I'll write something.

@farnoy farnoy force-pushed the update-scm-record branch from bd3fd04 to 07593c3 Compare October 14, 2024 23:04
@farnoy
Copy link
Contributor Author

farnoy commented Oct 14, 2024

Done

@ilyagr
Copy link
Contributor

ilyagr commented Oct 14, 2024

Once the wrapping is fixed, I'm planning to merge this in a few hours (to give people time to look, to potentially tell me not to do it, or for Martin to appear and give you the ability to do it yourself).

@farnoy farnoy force-pushed the update-scm-record branch from 07593c3 to 1dc4094 Compare October 14, 2024 23:50
@ilyagr ilyagr merged commit f741102 into jj-vcs:main Oct 15, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

diffedit blank lines on Windows Built-in diff editor doesn't redraw screen correctly in the presence of tabs
4 participants