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

Improve README #427

Merged
merged 6 commits into from
Dec 15, 2022
Merged

Improve README #427

merged 6 commits into from
Dec 15, 2022

Conversation

kaijchen
Copy link
Contributor

@kaijchen kaijchen commented Dec 15, 2022

What changes were proposed in this pull request?

  1. Update the introduction section in README:
    • "compute engines" is changed to "distributed compute engines".
    • "MapReduce" is changed to "Apache Hadoop MapReduce".
    • Remove the "Total lines" badge, see OSSInsight for better insight.
  2. Fix typos.

Why are the changes needed?

Improve the README.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Preview: https://github.com/kaijchen/incubator-uniffle/tree/readme#apache-uniffle-incubating

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022

Codecov Report

Merging #427 (27a0afe) into master (c8f8292) will decrease coverage by 0.80%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #427      +/-   ##
============================================
- Coverage     58.41%   57.61%   -0.81%     
+ Complexity     1612     1393     -219     
============================================
  Files           195      173      -22     
  Lines         11063     9223    -1840     
  Branches        976      816     -160     
============================================
- Hits           6463     5314    -1149     
+ Misses         4221     3573     -648     
+ Partials        379      336      -43     
Impacted Files Coverage Δ
...orage/LowestIOSampleCostSelectStorageStrategy.java 70.90% <0.00%> (-3.64%) ⬇️
.../java/org/apache/uniffle/server/ShuffleServer.java 64.49% <0.00%> (-2.90%) ⬇️
...storage/handler/impl/DataSkippableReadHandler.java 83.78% <0.00%> (-2.71%) ⬇️
...mapreduce/task/reduce/RssInMemoryRemoteMerger.java
.../java/org/apache/spark/shuffle/RssSparkConfig.java
.../hadoop/mapreduce/task/reduce/RssEventFetcher.java
...preduce/task/reduce/RssRemoteMergeManagerImpl.java
...ava/org/apache/spark/shuffle/RssShuffleHandle.java
.../java/org/apache/hadoop/mapreduce/RssMRConfig.java
.../org/apache/spark/shuffle/writer/WriterBuffer.java
... and 15 more

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

Copy link
Contributor

@advancedxy advancedxy left a comment

Choose a reason for hiding this comment

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

One minor comments, I'm fine with other modifications.

README.md Outdated

Coordinator will collect status of shuffle server and do the assignment for the job.
Coordinator will collect the status of the shuffle server and do the assignment for the job.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about this?

Coordinator will collect the status of the shuffle server and calculate shuffle assignments for compute job/app based on various assignment strategy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about "Coordinator will collect the status of shuffle servers and assign jobs based on some strategy."?

@kaijchen
Copy link
Contributor Author

In addition, I have tweaked the following sentence:

-Uniffle contains coordinator cluster, shuffle server cluster and remote storage(e.g., HDFS) if necessary.
+Uniffle cluster consists of three components, a coordinator cluster, a shuffle server cluster and an optional remote storage (e.g., HDFS).

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @kaijchen

@jerqi jerqi merged commit 7e7def8 into apache:master Dec 15, 2022
@kaijchen kaijchen deleted the readme branch December 15, 2022 12:32
kaijchen added a commit that referenced this pull request Jan 17, 2023
1. Update the introduction section in README:
    * "compute engines" is changed to "distributed compute engines".
    * "MapReduce" is changed to "Apache Hadoop MapReduce".
    * Remove the "Total lines" badge, see [OSSInsight][1] for better insight.
2. Fix typos.

[1]: https://ossinsight.io/analyze/apache/incubator-uniffle#lines-of-code-changed "apache/incubator-uniffle | OSSInsight"

Improve the README.

No.

Preview: https://github.com/kaijchen/incubator-uniffle/tree/readme#apache-uniffle-incubating
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

4 participants