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

Rename Chmod event to Metadata #112

Closed
alexbowers opened this issue Feb 20, 2017 · 6 comments
Closed

Rename Chmod event to Metadata #112

alexbowers opened this issue Feb 20, 2017 · 6 comments

Comments

@alexbowers
Copy link

When you touch a file, it currently files the chmod, which sounds incorrect (chmod is about permissions on *nix).

An alias to metadata would help code read a little better

@dfaust
Copy link
Collaborator

dfaust commented Feb 21, 2017

I like the idea. But adding another variant to DebouncedEvent is not a good solution because we would have to send metadata change events twice and every API user would have to handle them (or get a compiler warning). So we would have to replace the Chmod variant.

@alexbowers
Copy link
Author

Is deprecating the Chmod variant an option perhaps, and changing its name to Metadata in a future release?

@passcod passcod changed the title Consider DebouncedEvent::Metadata() as alias to Chmod Rename Chmod event to Metadata Feb 22, 2017
@passcod
Copy link
Member

passcod commented Feb 22, 2017

Yes. The chmod event is always about file metadata, and never limited to just permissions, so that's a good idea. This is both applicable to Chmod (in DebouncedEvent) and CHMOD (in Op).

I think we should mark those as deprecated and mention that they'll be renamed. Then the actual change should be done whenever the next major goes through. That might be a while, though.

@alexbowers
Copy link
Author

@passcod Has this been implemented? #114 mentions it was, but I can't see it in code.

@passcod
Copy link
Member

passcod commented Jun 27, 2017

Err. Hmm. No, I don't think I've done this. The spec in #117 is basically where I wanted to go, with the current release as a "final 4.x version". As in, I could deprecate this particular thing, but if the API is going to radically change anyway... 🤷

Unfortunately I've mostly lost interest in this project, and that plus being very busy at work and elsewhere has my attention gone. So notify is in a holding pattern of "it works."

@passcod
Copy link
Member

passcod commented Dec 28, 2017

This is done in v5, which also brings a different approach to event classification.

@passcod passcod closed this as completed Dec 28, 2017
@passcod passcod added this to the 5.0.0 milestone Dec 28, 2017
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

No branches or pull requests

3 participants