Skip to content

Make Logger.null public + minor clean up#2094

Merged
jsuereth merged 3 commits intosbt:0.13from
pdalpra:public-null-logger
Jul 10, 2015
Merged

Make Logger.null public + minor clean up#2094
jsuereth merged 3 commits intosbt:0.13from
pdalpra:public-null-logger

Conversation

@pdalpra
Copy link
Member

@pdalpra pdalpra commented Jul 9, 2015

This PR makes the dummy Logger.Null public.
While I was at it, I also added explicit types annotation on methods where they were missing.

@eed3si9n eed3si9n added the ready label Jul 9, 2015
@typesafe-tools
Copy link

typesafe-tools commented Jul 9, 2015

Can one of the admins verify this patch?

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

Shall I add notes too ?

@eed3si9n
Copy link
Member

eed3si9n commented Jul 9, 2015

Yes for notes. The change looks fine pending Travis.

@pdalpra
Copy link
Member Author

pdalpra commented Jul 9, 2015

Notes added, in 0.13.10.

jsuereth
jsuereth reviewed Jul 10, 2015
View reviewed changes
interface/src/main/java/xsbti/Logger.java
Copy link
Member

@jsuereth jsuereth Jul 10, 2015

Choose a reason for hiding this comment

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

why no public?

Copy link
Member Author

@pdalpra pdalpra Jul 10, 2015

Choose a reason for hiding this comment

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

In a Java interface, methods are implicitly public, constants are implicitly public static final, so public is not needed :)

Copy link
Member

@eed3si9n eed3si9n Jul 10, 2015

Choose a reason for hiding this comment

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

Found this on StackOverflow, JLS says http://docs.oracle.com/javase/specs/jls/se7/html/jls-9.html#jls-9.4

It is permitted, but discouraged as a matter of style, to redundantly specify the public and/or abstract modifier for a method declared in an interface.

@jsuereth
Copy link
Member

jsuereth commented Jul 10, 2015

LGTM

@pdalpra
Copy link
Member Author

pdalpra commented Jul 10, 2015

Shall I amend my PR to make the Null Logger to an object, like you suggested in the mailing list ?

@jsuereth
Copy link
Member

jsuereth commented Jul 10, 2015

@pdalpra No. Looks like it wouldn't be binary compatible then.

jsuereth added a commit that referenced this pull request Jul 10, 2015
@jsuereth
Merge pull request #2094 from pdalpra/public-null-logger
0e861c9 
Make Logger.null public + minor clean up
@jsuereth jsuereth merged commit 0e861c9 into sbt:0.13 Jul 10, 2015
@jsuereth jsuereth removed the ready label Jul 10, 2015
@pdalpra pdalpra deleted the public-null-logger branch February 29, 2016 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pdalpra @typesafe-tools @eed3si9n @jsuereth