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

Use java.time.Duration to represent time deltas instead of elementary types #611

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

mykola-mokhnach
Copy link
Contributor

Change list

Replace timeout/duration arguments where possible with java.time.Duration type, which has been introduced in Java 8.. This gives us better control over time conversion and helps to avoid time unit conversion issues in general.

All the existing methods, that have replacements, were marked as deprecated.

Types of changes

  • No changes in production code.
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

import java.time.Duration;

public class DurationUtils {
public static int durationToMillis(Duration duration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@mykola-mokhnach
Maybe it is more senseful to use this method instead of the util

Duration duration;
....
duration.toMillis();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. I dunno why I skipped the fact, that Duration class already has such method %)

@TikhomirovSergey
Copy link
Contributor

@mykola-mokhnach Hi. Sorry for the delaying. I am usually busy now.
These changes are cool 👍 But there is the question above. Cold you answer or make some improvements?

@SrinivasanTarget What is your opinion?

@mykola-mokhnach
Copy link
Contributor Author

@TikhomirovSergey np, thanks for the review. I'd be happy if someone could also add comments to the other one at #595

@TikhomirovSergey
Copy link
Contributor

@mykola-mokhnach Thank you very much!

@TikhomirovSergey TikhomirovSergey merged commit 2cc72ae into appium:master Apr 4, 2017
@mykola-mokhnach mykola-mokhnach deleted the duration_arg branch June 26, 2017 11:43
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.

3 participants