-
Notifications
You must be signed in to change notification settings - Fork 44
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
[IMP] autofill: support date autofill #5011
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.
Improving the autofill, this sounds easy 🙃
491296d
to
20a2ed0
Compare
src/helpers/dates.ts
Outdated
copy() { | ||
return DateTime.fromTimestamp(this.getTime()); | ||
} |
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 wonder if this is a good method. it looks like a new constructor, and has only 1 usage...
Also, it only copies the time.
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.
Nice :)
Might be worth merging in 18 ? What do you think ?
src/registries/autofill_rules.ts
Outdated
if (group.length === 2) { | ||
group.length * (group[1] - group[0]); | ||
} |
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.
looks like a useless if
: no side effect and no return
Task: 4173602
20a2ed0
to
ebab826
Compare
let's go for 18.0 :o |
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.
let's go for 18.0 :o
on est des fous nous 🙃
robodoo r+
Task: 4173602
Description:
description of this task, what is implemented and why it is implemented that way.
Task: TASK_ID
review checklist