Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Changed the default source and target java version to 1.7 :( #151

Merged
merged 1 commit into from Oct 30, 2017
Merged

Changed the default source and target java version to 1.7 :( #151

merged 1 commit into from Oct 30, 2017

Conversation

dxslly
Copy link
Collaborator

@dxslly dxslly commented Oct 29, 2017

Fix for #145

Downgraded the version of java we compile against from 1.8 to 1.7.

@codecov-io
Copy link

codecov-io commented Oct 29, 2017

Codecov Report

Merging #151 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #151      +/-   ##
============================================
- Coverage     72.31%   72.27%   -0.04%     
  Complexity      483      483              
============================================
  Files            65       65              
  Lines          2669     2669              
  Branches        248      248              
============================================
- Hits           1930     1929       -1     
- Misses          668      670       +2     
+ Partials         71       70       -1
Impacted Files Coverage Δ Complexity Δ
bbq/java/com/google/bbq/BroadcastQueryClient.java 54.68% <0%> (-0.79%) 7% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 859e4c1...605139e. Read the comment docs.

Copy link
Collaborator

@StanKocken StanKocken left a comment

Choose a reason for hiding this comment

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

Small comment, not a blocker.
LGTM

@@ -21,8 +21,8 @@ android {
}
}
compileOptions {
targetCompatibility 1.8
sourceCompatibility 1.8
targetCompatibility JavaVersion.VERSION_1_8
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why keep 1_8 here? It may be easier for developers to understand the code if they are used to (so not Java 1.8)

Copy link
Collaborator

Choose a reason for hiding this comment

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

++ unless there's a good reason to do otherwise, we should be consistent with the Java version

Copy link
Collaborator Author

@dxslly dxslly Oct 30, 2017

Choose a reason for hiding this comment

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

Given that is is was not part the public libraries I didn't see the need to remain consistent. In my mind the passwordlogin example was the "The latest and greatest practices" example, but there's no other motivation I can think of and you make a good point.

I will plan to migrate it in a follow up PR.

@dxslly dxslly merged commit 0ae3b1d into openid:master Oct 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants