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.

Sign up for GitHub

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

Jump to bottom

Try to remove SuppressWarnings("GuardedBy") #6578

Open
ejona86 opened this issue Dec 30, 2019 · 2 comments · May be fixed by #11620
Open

Try to remove SuppressWarnings("GuardedBy") #6578

ejona86 opened this issue Dec 30, 2019 · 2 comments · May be fixed by #11620
Labels
code health
Milestone
Next

Comments

@ejona86
Copy link
Member

ejona86 commented Dec 30, 2019

#6566 introduced suppressions to allow being compatible with newer versions of Error Prone. But we should spend some time to see if we could tweak the code so that the suppressions would be unnecessary.

If we can't remove them or if it would make the code worse, we can leave them as-is and close this issue.
@ejona86 ejona86 added this to the Next milestone Dec 30, 2019
@ejona86 ejona86 mentioned this issue Dec 30, 2019
Merged
dapengzhang0 pushed a commit that referenced this issue Dec 30, 2019
@graememorgan @dapengzhang0
all: Add suppressions for GuardedBy violations
d3c77f2 
This supports releasing a new version of GuardedBy which finds more mistakes than it used to.

Filed #6578 to try to clean up the suppressions.
@suztomo suztomo mentioned this issue Dec 30, 2019
Merged
rickwebiii pushed a commit to rickwebiii/grpc-java that referenced this issue Aug 5, 2020
@graememorgan @rickwebiii
all: Add suppressions for GuardedBy violations
b0dce23 
This supports releasing a new version of GuardedBy which finds more mistakes than it used to.

Filed grpc#6578 to try to clean up the suppressions.
@rickwebiii rickwebiii mentioned this issue Aug 5, 2020
Merged
dapengzhang0 pushed a commit that referenced this issue Aug 17, 2020
@rickwebiii @graememorgan
all: Add suppressions for GuardedBy violations (#7291)
bb47680 
This supports releasing a new version of GuardedBy which finds more mistakes than it used to.

Filed #6578 to try to clean up the suppressions.

Co-authored-by: Graeme Morgan <[email protected]>
dfawley pushed a commit to dfawley/grpc-java that referenced this issue Jan 15, 2021
@graememorgan @dfawley
all: Add suppressions for GuardedBy violations
254c13a 
This supports releasing a new version of GuardedBy which finds more mistakes than it used to.

Filed grpc#6578 to try to clean up the suppressions.
@vinodhabib
Copy link
Contributor

Started Analysis and going through details.

@vinodhabib vinodhabib linked a pull request Oct 16, 2024 that will close this issue
Open
@kannanjgithub
Copy link
Contributor

This comment for example, says

// TODO(b/145386688): This access should be guarded by 'stream.transportState().lock'; instead
// found: 'this.lock'

Why should it be guarded instead by stream.transportState().lock? What was the change in behavior that CL/282908895 introduced in ErrorProne? At least the above usage seems like would make the code worse if we don't guard it on this.lock that it does today. @ejona86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
code health
Projects
None yet
Milestone
Next
Development

Successfully merging a pull request may close this issue.

okhttp: Removed Annotation SuppressWarnings("GuardedBy") vinodhabib/grpc-java
3 participants
@ejona86 @kannanjgithub @vinodhabib