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

refactor: fix SonarQube - make field 'private', use accessor #2387

Merged
merged 3 commits into from
Aug 20, 2018
Merged

refactor: fix SonarQube - make field 'private', use accessor #2387

merged 3 commits into from
Aug 20, 2018

Conversation

zielint0
Copy link
Contributor

@zielint0 zielint0 commented Aug 17, 2018

Change access level from 'public' to 'private'.
Use accessor in other places.

I even did not add an accessor - it existed so I used it. I assume that this public accessor (getContext()) was added to have this field private. Should not be a compatibility issue.

@spoon-bot
Copy link
Collaborator

API changes: 1 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:7.1.0-20180809.071946-110 / New API: fr.inria.gforge.spoon:spoon-core:jar:7.1.0-SNAPSHOT

Visibility was reduced from 'public' to 'private'.
Old field DefaultJavaPrettyPrinter.context
New field DefaultJavaPrettyPrinter.context
Breaking binary: breaking,

@surli
Copy link
Collaborator

surli commented Aug 19, 2018

I assume that this public accessor (getContext()) was added to have this field private. Should not be a compatibility issue.

I agree this need a change however it could be a compatibility issue for users that inherits from DefaultJavaPrettyPrinter and uses the field context.
I propose that you add a deprecated annotation on it with a since 7.1.0. And add some javadoc to explain to use the getter instead. It will be removed in the next two releases.

@zielint0
Copy link
Contributor Author

@surli

I propose that you add a deprecated annotation on it with a since 7.1.0. And add some javadoc to explain to use the getter instead. It will be removed in the next two releases.

Done. Please verify it.

@surli surli merged commit e4e2a02 into INRIA:master Aug 20, 2018
@monperrus monperrus mentioned this pull request Sep 20, 2018
4 tasks
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