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: Implement jj git sync #4641

Closed
essiene opened this issue Oct 14, 2024 · 4 comments
Closed

FR: Implement jj git sync #4641

essiene opened this issue Oct 14, 2024 · 4 comments
Assignees
Labels
duplicate This issue or pull request already exists

Comments

@essiene
Copy link
Collaborator

essiene commented Oct 14, 2024

This is done as part of #1039, focusing only on jj git sync which is has agreement on proceeding. Summary from conclusion thread in #1039 is below:

Summary:
Ideally jj sync should be a un-namespaced command (without a backend prefix) but this makes it difficult to determine which implementation to use. @martinvonz's suggestion in an internal chat and on Discord was to make the command forge specific, e.g jj git sync, jj gerrit sync or jj piper sync and then allow the unnamespaced sync command to wrap it. This means that for the most users, jj sync would be jj git sync. Another option is to make that a buiiltin alias for each backend.

While I was initially sceptical of the namespaced approach, I now think it is the right solution for this problem. The only issue I see, if we take the forge-specific approach, is that some layering issue may pop up. But if we implement something like jj submit or jj mail, a forge trait is needed anyway.

Originally posted by @PhilipMetzger in #1039 (comment)

@essiene essiene self-assigned this Oct 14, 2024
@martinvonz
Copy link
Member

I'm not sure we need this separate FR (but I don't really mind it either). Will there be anything left in #1039 once this one has been implemented?

@essiene
Copy link
Collaborator Author

essiene commented Oct 14, 2024

I'm not sure we need this separate FR (but I don't really mind it either). Will there be anything left in #1039 once this one has been implemented?

I'm not sure if there will still be work left to define a platform support so that a future jj sync invokes the forge specific sync action, if it exists.

So I didn't want to kill off the conversation by closing that issue when jj git sync is done.

wdyt?

@martinvonz
Copy link
Member

I personally don't think we should define a top-level, forge-independent jj sync, so I think that issue should be closed when this one is closed.

@essiene
Copy link
Collaborator Author

essiene commented Oct 14, 2024

I personally don't think we should define a top-level, forge-independent jj sync, so I think that issue should be closed when this one is closed.

My bad. I misunderstood the conclusion of that thread. I'll just close this and assign the other to myself to not split the conversation history.

Thanks for clarifying.

@essiene essiene closed this as completed Oct 14, 2024
@essiene essiene added the duplicate This issue or pull request already exists label Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

2 participants