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

FR: Add new revset builtin_immutable_heads() #4195

Merged
merged 1 commit into from
Aug 14, 2024
Merged

FR: Add new revset builtin_immutable_heads() #4195

merged 1 commit into from
Aug 14, 2024

Conversation

essiene
Copy link
Contributor

@essiene essiene commented Aug 1, 2024

  • Define builtin_immutable_heads() in default config layer.
  • Redefine immutable_heads() in terms of builtin_immutable_heads()
  • Issue warnings if any of builtin_immutable_heads(), mutable() or immutable() are redefined by the user.

Fixes: #4162

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have added tests to cover my changes

Copy link
Contributor Author

@essiene essiene left a comment

Choose a reason for hiding this comment

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

SELF REMINDER: squash commits before submitting.

Done.

Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

I think we need a way to override trunk() since some repos have main branches called something other than main, master or trunk.

Done.

@essiene essiene force-pushed the fr/4162 branch 3 times, most recently from 086dced to 4f77a73 Compare August 2, 2024 00:11
@essiene
Copy link
Contributor Author

essiene commented Aug 2, 2024

I think we need a way to override trunk() since some repos have main branches called something other than main, master or trunk.

I've changed my approach. Instead of defining this in jj-lib, I'm now defining it as an alias in a new config source layer that takes precedence over all other config sources: builtins.toml

So since it's just a regular alias, it can be defined in terms of trunk() with no issues, as opposed to my previous approach.

wdyt?

If this looks good, I'll go ahead to add some tests.

@essiene essiene marked this pull request as ready for review August 2, 2024 00:24
@essiene essiene changed the title draft: FR: Add new revset builtin_immutable_heads() FR: Add new revset builtin_immutable_heads() Aug 2, 2024
cli/src/config.rs Outdated Show resolved Hide resolved
@essiene essiene force-pushed the fr/4162 branch 10 times, most recently from ab4d64f to 89907b3 Compare August 11, 2024 19:36
@essiene essiene requested review from yuja and martinvonz August 11, 2024 19:59
cli/src/config.rs Outdated Show resolved Hide resolved
cli/src/config.rs Outdated Show resolved Hide resolved
@essiene essiene force-pushed the fr/4162 branch 3 times, most recently from 22f3838 to 10dfa89 Compare August 13, 2024 19:14
@essiene essiene requested a review from yuja August 13, 2024 19:20
@essiene essiene assigned essiene and unassigned essiene Aug 13, 2024
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

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

Thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
cli/src/revset_util.rs Outdated Show resolved Hide resolved
cli/src/revset_util.rs Outdated Show resolved Hide resolved
cli/src/revset_util.rs Show resolved Hide resolved
* Add `builtin_immutable_heads()` in the `revsets.toml`.
* Redefine `immutable_heads()` in terms of `builtin_immutable_heads()`
* Warn if user redefines `builtin_immutable_heads()`, `mutable()` or
  `immutable()`.
* Update module constant in revset_util.rs from BUILTIN_IMMUTABLE_HEADS
  to USER_IMMUTABLE_HEADS to avoid confusion since it points at
  `immutable_heads()` **and** we now have a revset-alias
  literally named `builtin_immutable_heads()`.
* Add unittest
* Update CHANGELOG
* Update documentation.

Fixes: #4162
@essiene
Copy link
Contributor Author

essiene commented Aug 14, 2024

Thanks!

Yw, and thanks again for the patient explanations. Much appreciated.

@essiene essiene merged commit a6d8009 into main Aug 14, 2024
29 checks passed
@essiene essiene deleted the fr/4162 branch August 14, 2024 10:32
@ilyagr

This comment was marked as duplicate.

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.

FR: Add builtin_immutable_heads()
4 participants