Skip to content

Add rule Handling Date/Time #5

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add rule Handling Date/Time #5

wants to merge 3 commits into from

Conversation

vpon-jiri
Copy link

No description provided.

@@ -633,6 +633,27 @@ log.error(f"An exception was thrown for value $value%.12f: $ex")

Do not nest `map` more than twice. Use named values/functions instead.

## Handling Date And Time

It is strongly recommended to use time library (either `java.time` or Joda Time) instead of doing calculations with milliseconds. As one implication of this rule, using System.currentTimeMillis()` is strongly discouraged and should not be used for time operations.
Copy link
Author

Choose a reason for hiding this comment

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

Kevin says: Please make a clear statement for when to use unix timestamp, and when to use date/time library. When serialize and store to DB, unix timestamp is the only choice with no exception. date/time library should only be used when cross-day time manipulation is required.

@vpon-jiri
Copy link
Author

Please make a clear statement for when to use unix timestamp, and when to use date/time library. When serialize and store to DB, unix timestamp is the only choice with no exception. date/time library should only be used when cross-day time manipulation is required.

I intentionally used words like operations, arithmetic and calculations to stress that the usage of time library is strongly recommended in those cases. From my point of view, in Scala code unix time should never be used directly. Or can you give some example when using unix time in Scala code is justified? I can add an explicit mention about DB.

@quadas-yansu
Copy link

good to merge

@vpon-jiri
Copy link
Author

I can't yet, there is no general agreement on this.

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.

2 participants