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

Calculate zoneOffset based on zoneId in TestActorRequestFactory #915

Merged
merged 4 commits into from
Dec 13, 2018

Conversation

serhii-lekariev
Copy link
Contributor

@serhii-lekariev serhii-lekariev commented Dec 13, 2018

This PR addresses #885.

Before, to create an instance of TestActorRequestFactory, two timezone-related parameters were needed:
zone ID and zone offset.

Since it is possible to calculate offset based on ID, two parameters have been substituted with a single one - zone ID. Zone offset is now calculated based on the ID.

@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

Merging #915 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #915      +/-   ##
============================================
+ Coverage     93.38%   93.38%   +<.01%     
- Complexity     3868     3869       +1     
============================================
  Files           529      529              
  Lines         12629    12637       +8     
  Branches        703      703              
============================================
+ Hits          11793    11801       +8     
  Misses          607      607              
  Partials        229      229

@serhii-lekariev
Copy link
Contributor Author

@dmdashenkov, PTAL.

Copy link
Contributor

@dmdashenkov dmdashenkov left a comment

Choose a reason for hiding this comment

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

@serhii-lekariev, LGTM with a single comment to address.

@@ -98,6 +100,19 @@ public static TestActorRequestFactory newInstance(Class<?> testClass, TenantId t
ZoneIds.systemDefault());
}

private static ZoneOffset idToZoneOffset(ZoneId zoneId) {
String id = zoneId.getValue();
java.time.ZoneId javaZoneId = java.time.ZoneId.of(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please inline the zoneId.getValue() call. The id variable doesn't make the code more readable, since "ID" is too broad of a term.

@serhii-lekariev serhii-lekariev merged commit 4d39747 into master Dec 13, 2018
@serhii-lekariev serhii-lekariev deleted the calculate-zone-offset-from-id branch December 13, 2018 13:16
@armiol armiol mentioned this pull request Dec 20, 2018
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