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

Add DateTime Delta Operation #1732

Merged
merged 11 commits into from
Apr 2, 2024
Merged

Add DateTime Delta Operation #1732

merged 11 commits into from
Apr 2, 2024

Conversation

tomgond
Copy link
Contributor

@tomgond tomgond commented Feb 21, 2024

Adds DateTime Delta operations.
This allows to calculate a new DateTime given an input DateTime and the amount of time passed / before the input DateTime.
For example:
Input: 20/02/2024 13:36:00
Difference: +0.00:01:00
Output: 20/02/2024 13:37:00

@a3957273
Copy link
Member

I've gone back to this PR half a dozen times and, umming and erring as to whether to approve it or not. The operator works (apart from a few linting issues caught by eslint). The parsing of dates is normal and supports a wide variety of formats.

My issue is with the 'delta' argument. It is very brittle and needs to be in that specific format. I wonder if we could replace it with a better format? I can't find any great examples online to use. The 'moment' library accepts partial times (e.g. 12:13:14) but parses them to the current day, which isn't useful.

I'm open to suggestions / opposition, but I don't think the current format is suitable. Some options off the top of my head:

  • A time period like "15 seconds", "23 minutes" is used by some libraries.
  • Input seconds (e.g. 86400 for a day).
  • Enter in a string and it's format (e.g. '00:10:22' and 'hh:mm:ss').

@tomgond
Copy link
Contributor Author

tomgond commented Mar 31, 2024

Hey, thanks for the input.
I understand what you are saying, the delta parameter is indeed quite specific, but I think it's the best way to represent this operation.
What if instead of delta being a string, I'll make a textbox for each time unit (one for days, hours, minutes, seconds, and if we add or substract time). That way it will be more obvious and less brittle, but still keeps the format.

If not, I'm open to implement other changes. Can you give an example of time period? can this be like "5 hours, 32 minutes and 3 seconds?"

@a3957273
Copy link
Member

I like your idea of separating out the time units into each individual text box! That feels like a way better solution. ❤️

@a3957273
Copy link
Member

a3957273 commented Apr 1, 2024

Yell when this is ready to re-review :)

@tomgond
Copy link
Contributor Author

tomgond commented Apr 2, 2024

Hey, I think it's ready :)

Copy link
Member

@a3957273 a3957273 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed output format to HTML so that the 'FORMAT_EXAMPLES' string renders correctly. Other than that, this looks really good. Thanks for putting the effort into making this, I've wanted this precise operator before!

Screenshot 2024-04-02 at 21 28 16

@a3957273 a3957273 merged commit 2000938 into gchq:master Apr 2, 2024
4 checks passed
@tomgond
Copy link
Contributor Author

tomgond commented Apr 2, 2024

Glad to give back for this great tool. Actually I had similar operation in mind if this goes well (as it did) which is time difference, which is kinda the opposite operation from this one (given two dates, how much time passed between). Any special consideration for this one?

@tomgond tomgond deleted the date-delta branch April 3, 2024 05:37
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

Successfully merging this pull request may close these issues.

None yet

2 participants