Skip to content

Conversation

malcolmtaylor
Copy link
Contributor

Motivation

LGTM has raised some alerts during analysis of apache/pulsar (https://lgtm.com/projects/g/apache/pulsar/alerts/?mode=tree).

Modifications

  • Fixed 1 Array index out of bounds error in AbstractMetrics
  • Fixed 3 cases of Hashed value without hashCode definition in NamespaceService
  • Fixed 1 Reference equality test of boxed types in Policies
  • Fixed 1 Wrong NaN comparison in ProducerStatsRecorderImpl
  • Fixed 16 cases of Inconsistent equals and hashCode where equals had been overridden but hashCode was not

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API: ( no)
  • The schema: ( no )
  • The default values of configurations: (no)
  • The wire protocol: ( no)
  • The rest endpoints: (no)
  • The admin cli options: ( no)
  • Anything that affects deployment: (no )

Documentation

  • Does this pull request introduce a new feature? ( no)

@merlimat merlimat added type/bug The PR fixed a bug or issue reported a bug type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Feb 6, 2019
@merlimat merlimat added this to the 2.3.0 milestone Feb 6, 2019
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@merlimat
Copy link
Contributor

merlimat commented Feb 7, 2019

retest this please

@merlimat
Copy link
Contributor

merlimat commented Feb 9, 2019

run java8 test

1 similar comment
@sijie
Copy link
Member

sijie commented Feb 11, 2019

run java8 test

@sijie
Copy link
Member

sijie commented Feb 11, 2019

run java8 tests

@malcolmtaylor
Copy link
Contributor Author

It is not clear to me why that test is failing. ReplicatorTest passed for me locally, and after rebasing against master it still passed. It also doesn't look like something that would be related to the changes I made.

@merlimat merlimat modified the milestones: 2.3.0, 2.4.0 Feb 14, 2019
@sijie
Copy link
Member

sijie commented Feb 20, 2019

@1m2c3t4 replicator test is a flaky test.

// run java8 tests

@sijie
Copy link
Member

sijie commented Mar 19, 2019

run java8 tests

@sijie sijie merged commit c180bbc into apache:master Mar 20, 2019
@malcolmtaylor
Copy link
Contributor Author

Thanks for reviewing and merging @sijie @merlimat @rdhabalia

@malcolmtaylor malcolmtaylor deleted the fix-lgtm-alerts branch March 20, 2019 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants