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

jujutsu: support darwin guidelines for config placement #5781

Conversation

jkachmar
Copy link

Description

backports changes from #5416 to the release-24.05 branch.

Maintainer CC

@shikanime

Follow up to nix-community#5207, fixing jujutsu module on darwin targets.
@jkachmar
Copy link
Author

jkachmar commented Sep 1, 2024

how does merging work for home-manager anyway? @rycee merged the original PR, but the UI says we should be able to merge this with an approval from a maintainer.

@rycee
Copy link
Member

rycee commented Sep 7, 2024

Thanks! Rebased to the 24.05 branch in 3288b74.

@rycee rycee closed this Sep 7, 2024
@puppe
Copy link

puppe commented Sep 13, 2024

I appreciate this change in the grand scheme of things. With jj ignoring XDG_CONFIG_HOME, this is definitely the right thing to do. But as I already had a workaround in place by setting JJ_CONFIG, this just broke my setup. It took me embarassingly long to find the cause (in part because I had forgotten about setting JJ_CONFIG).

Personally, I would have preferred if this would not have been backported. If this had come with the next release with an accompanying entry in the release notes, that would probably have saved me a lot of time. (I hope I do not sound too cranky. It is getting late in UTC+0200.)

@rycee
Copy link
Member

rycee commented Sep 15, 2024

@puppe Sorry about the wasted time! I think you have a good point about this being a breaking change and it shouldn't have made it into the stable branch.

Maybe it still would have been valuable to bring into the stable branch but perhaps with a news entry conditioned on jujutsu being enabled, then you at least would be notified about the change.

In any case, I will keep this in mind for the future.

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