-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add new snippets #25
Add new snippets #25
Conversation
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.
The documentation looks good, but you forgot to add the actual snippets in snippets/liquid-template-snippets.json
😅
38830fd
to
b1e4ffd
Compare
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.
Added the comments on the snippets code
"body": ".star", | ||
"description": "Return all starred reconciliations - must use with variable of type reconciliations (drop)" | ||
}, | ||
"Reconciliation star drop (with options)": { |
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.
This should be starred?
?
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.
Seems to just be 'starred' per guidance? (https://developer.silverfin.com/docs/reconciliations), but have otherwise updated
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.
You're right, I was looking at the ReconciliationDrop (https://developer.silverfin.com/docs/reconciliation) instead of the ReconciliationsDrop
"prefix": "date: YYYY-MM-DD", | ||
"body": "| date:\"%F\"", | ||
"description": [ | ||
"ISO date formatting (DD/MM/YYYY).\n", |
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.
Description should still be updated here as well
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.
Updated
"Date filter - DD/MM/YYYY format": { | ||
"prefix": "date: DD/MM/YYYY", | ||
"body": "| date:\"%d/%m/%Y\"", | ||
"description": "Standard BE date formatting (DD/MM/YYYY)." |
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.
Description should still be updated here as well
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.
Updated
"body": ".star", | ||
"description": "Return all starred reconciliations - must use with variable of type reconciliations (drop)" | ||
}, | ||
"Reconciliation star drop (with options)": { |
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.
You're right, I was looking at the ReconciliationDrop (https://developer.silverfin.com/docs/reconciliation) instead of the ReconciliationsDrop
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.
Some minor comments but the rest look good and can go live once the last few things are updated
Don't forget to also still bump the version in package.json to 1.17.0, run npm install to update the package-lock.json and update the changelog.md before packaging it for release |
Description
Update readme with details of drops
Fixes #5 (#5)
Type of change
Checklist
Notes