Closed xynydev closed 1 month ago
Ok, I fixed the actual issues I created #236 about here, but there's still some others we should discuss about I think @fiftydinar
install-chezmoi
key? It's not likely someone would want to use this module without having chezmoi installed, as that wouldn't work at all.enable-all-users
just be all-users
? I think that would make more sense.changed-file-policy
? I think the key name should be the same in the upcoming homefiles
module and here. existing-file-policy
wouldn't work in this context IMO, but file-conflict-policy
feels clear and would work.@xynydev
Do we need an install-chezmoi key? It's not likely someone would want to use this module without having chezmoi installed, as that wouldn't work at all.
I would agree to not need this & default it to true.
Should enable-all-users just be all-users? I think that would make more sense.
I agree
Should we change changed-file-policy? I think the key name should be the same in the upcoming homefiles module and here. existing-file-policy wouldn't work in this context IMO, but file-conflict-policy feels clear and would work.
I agree.
Should we break the change right of the bat or retain compatibility with changed-file-policy
?
I can also change it to file-conflict-policy
in the homefiles module.
Should we break the change right of the bat or retain compatibility with changed-file-policy?
Since nobody complained about the docs for the module being wrong before, I don't think there's that many people using it. I'd just rip off the band-aid and make the breaking change.
@exponentactivity since you're probably using your own module, you might want to know about these changes.
Otherwise, we're ready to merge now. (after an approving review)
There are breaking changes, because I don't think this module has many users, and the review process let through some subpar apples here.
this apparently was just a documentation problem, not an actual issue