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

sync: Consider renaming the command to match what it does #465

Closed
SaschaMann opened this issue Nov 9, 2021 · 4 comments
Closed

sync: Consider renaming the command to match what it does #465

SaschaMann opened this issue Nov 9, 2021 · 4 comments
Labels
cmd: sync kind: design Discussing overall direction status: wont do/fix This will not be worked on

Comments

@SaschaMann
Copy link
Contributor

sync doesn't sync anything, it checks if things are synced. The option that does sync appears to be -u.

I think configlet check-sync would be a more appropriate name, with configlet sync doing what configlet sync -u does currently.

@SaschaMann SaschaMann changed the title sync: Consider renaming the command to match its function sync: Consider renaming the command to match what it does Nov 9, 2021
@ErikSchierboom
Copy link
Member

I disagree with this. It is very common for linting/formatting tools to by default not write things. Case in points are rubocop , dotnet format and prettier. Only when passing in an additional flag will things actually be updated. I would find two separate commands to be confusing.

@SaschaMann
Copy link
Contributor Author

SaschaMann commented Nov 9, 2021

I would find two separate commands to be confusing.

One command is fine as well, but it should do what the name suggests, imo. Even configlet syncing or whatever would work better because it's not imperative, unlike sync. From those examples I only know prettier where the default command is just prettier, not prettier format (the latter, because it's in imperative mood, imo implies that it changes things).

configlet generate is an example that's part of configlet that lives up to its name and, afaict from the docs, writes by default.

@ee7 ee7 added kind: design Discussing overall direction cmd: sync labels Apr 1, 2022
@ee7
Copy link
Member

ee7 commented Apr 6, 2022

I never got around to replying to this, but I wasn't unsympathetic to the complaint.

I even wrote in the original design discussion #179 (comment):

if sync isn't allowed to write to the track repo unless --update is passed then maybe sync is a bad name.

However, I'm happy with the current interface for sync (and fmt). And it's true that configlet generate still works like the old implementation of sync - we're planning to make it consistent with how the other commands now work.

@ee7
Copy link
Member

ee7 commented Jul 1, 2022

And it's true that configlet generate still works like the old implementation of sync - we're planning to make it consistent with how the other commands now work.

This will happen with #619.

At this point, we don't have any plans to rename sync. And we've got #542 for tracking "make generate consistent with the other commands" so I don't think there's anything for this issue to track - I'm closing it. Please reopen if I've missed something. Thanks for creating this issue.

@ee7 ee7 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2022
@ee7 ee7 added the status: wont do/fix This will not be worked on label Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd: sync kind: design Discussing overall direction status: wont do/fix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants