Skip to content

Conversation

@satybald
Copy link
Contributor

What is the purpose of the change

Removing nested IFs for StreamRecord#equals method without changing the comparison logic.

Brief change log

  • refactors StreamRecord#equals method

Verifying this change

This change is already covered by existing tests, such as StreamRecordTest.

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

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@StephanEwen
Copy link
Contributor

Thank you joining the Flink community and contributing.

Considering this specific change: I would like to not merge this change.

There was nothing wrong with the original code - this is a cosmetic change that does not actually change anything significant. I personally would like to encourage not doing such changes, because a project can easily get into a situation where committers spend a significant amount of time formatting code and style back and forth rather than working on issues for noticeable improvements.

I would encourage you to look at the "how to contribute" page, which describes how to find interesting issues for new contributions.

@satybald
Copy link
Contributor Author

thanks for the feedback @StephanEwen

@satybald satybald closed this Jan 22, 2018
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.

2 participants