Skip to content

docs: add diagrams for jj new -A and -B #5635

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

Merged
merged 1 commit into from
Apr 10, 2025
Merged

Conversation

neongreen
Copy link
Contributor

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

@neongreen
Copy link
Contributor Author

image

@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch 3 times, most recently from 5c6920d to 04e0c96 Compare February 10, 2025 03:37
@neongreen
Copy link
Contributor Author

neongreen commented Feb 10, 2025

I realize now that jj prev and jj parallelize show graphs vertically and not horizontally, but also I think horizontal graphs look nicer (the real argument) and save space in the CLI output (the objective argument).

I don't mind converting horizontal->vertical or vice versa, but I also don't mind leaving them in an inconsistent state and seeing which will eventually win.

@yuja
Copy link
Contributor

yuja commented Feb 10, 2025

Can you rewrite the graph without using non-ASCII characters? Unicode drawings might be rendered badly. I don't have any preference over horizontal/vertical.

@PhilipMetzger
Copy link
Contributor

I agree with Yuya and think that the graph should be kept vertical to fit into the rest of the documentation as consistency is key.

@martinvonz
Copy link
Member

I agree with Philip that the graphs should be consistent with our existing graphs to make it easier for users to read them.

I don't have much opinion between horizontal vs vertical per se (maybe a slight preference for vertical since it matches jj log). I care more about the consistency.

@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from 04e0c96 to dc81c28 Compare April 7, 2025 21:03
@neongreen neongreen requested a review from a team as a code owner April 7, 2025 21:03
@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from dc81c28 to b521357 Compare April 7, 2025 21:10
@neongreen
Copy link
Contributor Author

I removed unicode and transmuted the diagrams

@PhilipMetzger
Copy link
Contributor

last concistency nit from me, use the letters instead of numbers in the graph

@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from b521357 to 5c76d17 Compare April 8, 2025 21:53
@neongreen
Copy link
Contributor Author

last concistency nit from me, use the letters instead of numbers in the graph

done, and I switched to --after and --before because -A A isn't great

@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from 5c76d17 to ac452fc Compare April 8, 2025 21:55
@neongreen neongreen enabled auto-merge April 9, 2025 07:22
@neongreen
Copy link
Contributor Author

Hmmm. Anything I need to do to start the CI? Or is it just that the merge queue is long?

@emilazy
Copy link
Contributor

emilazy commented Apr 9, 2025

You need to rebase this, it’s from before the CI changes so the mandatory job is never going to happen.

@neongreen neongreen disabled auto-merge April 10, 2025 07:58
@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from ac452fc to b4627b7 Compare April 10, 2025 07:59
@neongreen neongreen added this pull request to the merge queue Apr 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 10, 2025
@neongreen neongreen force-pushed the emily/jj-yksnuprrtowq branch from b4627b7 to 34d9d8b Compare April 10, 2025 08:16
@neongreen neongreen added this pull request to the merge queue Apr 10, 2025
Merged via the queue into main with commit 8c50600 Apr 10, 2025
28 checks passed
@neongreen neongreen deleted the emily/jj-yksnuprrtowq branch April 10, 2025 19:11
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.

5 participants