Skip to content

Conversation

@sahitya-pavurala
Copy link
Contributor

Should be logical(&&) and not bit wise(&)

Should be logical(&&) and not bit wise(&)
@sahitya-pavurala
Copy link
Contributor Author

It looks like I deleted my local repository accidentally. Let me know if I have to make this change again.

@fhueske
Copy link
Contributor

fhueske commented Jun 18, 2015

Thanks für the PR.
As long as you keep the PR branch in your GH repository, things are fine.

@sahitya-pavurala
Copy link
Contributor Author

okay thanks , so after you merge the code , there are few more things to be changed in those classes , will make the changes shortly.

@uce
Copy link
Contributor

uce commented Jun 18, 2015

Since both are booleans, this check is equivalent to && w/o the short-circuit evaluation. So good to merge.

@sahitya-pavurala
Copy link
Contributor Author

I am relatively new to github, i cant close the pr until you merge the code right?

@hsaputra
Copy link
Contributor

Hi @sahitya-pavurala , if the PR is merged we will automatically close the PR for you.

@sahitya-pavurala
Copy link
Contributor Author

Okay, thanks.

@StephanEwen
Copy link
Contributor

I think that the correct thing is to drop the check for not null, because the instanceof keyword checks that the argument is not null already. The current code hence checks twice that the value is not null.

@sahitya-pavurala
Copy link
Contributor Author

I think that the instanceOf should be changed to getClass in the first place. getClass makes more sense since its a equals method and we want both the types to be identical(not allow sub types)

@uce
Copy link
Contributor

uce commented Jun 22, 2015

Will squash with the other PR and merge.

@asfgit asfgit closed this in 3406d96 Jun 22, 2015
nikste pushed a commit to nikste/flink that referenced this pull request Sep 29, 2015
nltran pushed a commit to nltran/flink that referenced this pull request Jan 8, 2016
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.

5 participants