-
Notifications
You must be signed in to change notification settings - Fork 13
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
[VO-1032] feat: Update iocozyfile typings #1516
Conversation
packages/cozy-client/src/types.js
Outdated
@@ -441,6 +441,7 @@ import { QueryDefinition } from './queries/dsl' | |||
* @property {string} [noticePeriod] - Notice period of the paper, in days | |||
* @property {string} [datetime] - Image EXIF date, if relevant | |||
* @property {string} [instanceName] - Name of the instance | |||
* @property {string} [type] - Type of the file (mainly used for shortcuts categories) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that the description can be misleading since the file already has a type attribute but I don't know who to name it better 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading indeed, and also not documented, I don't see any mention here: https://github.com/cozy/cozy-doctypes/blob/master/docs/io.cozy.files.md nor here: https://github.com/cozy/cozy-doctypes/blob/master/docs/io.cozy.files_metadata.md
Where does it come from @acezard ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paultranvan it comes from custom shortcuts created by the toutatice konnector. they don't follow the files doctype to the letter. thus maybe it should not be typed into cozy-client and only locally in cozy-store? not sure. cc @cballevre
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be more in favour of the team going all the way, even if it means asking the konnector team to document it in cozy-doctypes, as it should have done initially, so you don't hide information by omitting it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they don't follow the files doctype to the letter
There is absolutely no reason to not follow the rules, especially in this central doctype that is used by many apps. Either it's documented in cozy-doctypes and typed in cozy-client, either it should not exist, there should be no in-between situation.
@acezard please, ask the guys in charge of the toutatice connector to make a PR on cozy-doctypes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm scribing for the team here's the PR with the new documentation maybe which can help refine the description here
This commit takes into account the decision on the metadata format for shortcuts taken as part of this PR : cozy/cozy-doctypes#268
884b64c
to
e84cfbb
Compare
After discussions in this PR we have converged on a new data format. This PR implements these changes in the cozy-client types 🙂 @paultranvan if you want to take a look 🙏 I would appreciate |
No description provided.