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

Add a reversed flag to jj op log #4301

Closed
wants to merge 1 commit into from

Conversation

lukeyeh
Copy link

@lukeyeh lukeyeh commented Aug 19, 2024

Checklist

If applicable:

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

In order to resolve #4190.

|OperationByEndTime(op)| op.parents().map_ok(OperationByEndTime).collect_vec(),
)
} else {
dag_walk::topo_order_reverse_ok(
Copy link
Contributor

Choose a reason for hiding this comment

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

Dropping lazy version is undesired. Maybe better to handle --reversed by caller in the same way as cmd_log(). I think the ancestors iterator can be mapped to (OperationId, Vec<GraphEdge<OperationId>>) or (Operation, Vec<GraphEdge<Operation>>) which is compatible with ReverseGraphIterator.

It might make sense to reimplement ReverseGraphIterator to support (T, Vec<GraphEdge<Id>>) pair.

@zx8
Copy link

zx8 commented Jan 3, 2025

@JDSeiler No pressure at all, but since you recently implemented #5170 and this PR hasn't seen any activity in ~4 months, while it's all still fresh in your mind, is this something you might consider taking on? 👀

@JDSeiler
Copy link
Contributor

JDSeiler commented Jan 3, 2025

@JDSeiler No pressure at all, but since you recently implemented #5170 and this PR hasn't seen any activity in ~4 months, while it's all still fresh in your mind, is this something you might consider taking on? 👀

Sure! The code looks similar enough to the log and evolog commands. My holiday time is about to run out and I'll be going back to my day job, so no guarantees on time-frame.

@bnjmnt4n
Copy link
Member

Just a heads up that I've opened #5329 for this.

@bnjmnt4n
Copy link
Member

Closed by #5329.

@bnjmnt4n bnjmnt4n closed this Jan 12, 2025
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 --reversed option to op log
5 participants