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

[Minor] Make Constants final #501

Merged
merged 2 commits into from
Jan 19, 2023
Merged

[Minor] Make Constants final #501

merged 2 commits into from
Jan 19, 2023

Conversation

kaijchen
Copy link
Contributor

@kaijchen kaijchen commented Jan 19, 2023

What changes were proposed in this pull request?

  1. Make Constants class final.
  2. Make public static fields in Constants final.
  3. Make default constructor of Constants private.

Why are the changes needed?

Code quality.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing CI.

zuston
zuston previously approved these changes Jan 19, 2023
Copy link
Member

@zuston zuston left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2023

Codecov Report

Merging #501 (c44df83) into master (9646a6f) will increase coverage by 0.75%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #501      +/-   ##
============================================
+ Coverage     59.34%   60.09%   +0.75%     
+ Complexity     1747     1615     -132     
============================================
  Files           206      192      -14     
  Lines         11508    10224    -1284     
  Branches       1030      908     -122     
============================================
- Hits           6829     6144     -685     
+ Misses         4274     3705     -569     
+ Partials        405      375      -30     
Impacted Files Coverage Δ
...pache/hadoop/mapreduce/task/reduce/RssFetcher.java
...n/java/org/apache/hadoop/mapreduce/RssMRUtils.java
...pache/hadoop/mapreduce/task/reduce/RssShuffle.java
.../java/org/apache/hadoop/mapreduce/RssMRConfig.java
...java/org/apache/hadoop/mapred/SortWriteBuffer.java
...apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
...n/java/org/apache/hadoop/mapreduce/MRIdHelper.java
...mapreduce/task/reduce/RssInMemoryRemoteMerger.java
...rg/apache/hadoop/mapred/RssMapOutputCollector.java
.../hadoop/mapreduce/task/reduce/RssBypassWriter.java
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kaijchen kaijchen merged commit 359f7cd into apache:master Jan 19, 2023
@kaijchen
Copy link
Contributor Author

Thanks @zuston for the review.

@kaijchen kaijchen deleted the final2 branch January 19, 2023 05:09
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.

None yet

3 participants