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

[CELEBORN-877][DOC] Document on SBT #1795

Closed
wants to merge 19 commits into from
Closed

Conversation

cfmcgrady
Copy link
Contributor

What changes were proposed in this pull request?

As title

Why are the changes needed?

As title

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manual test

@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #1795 (b15425a) into main (6ea1ee2) will increase coverage by 0.16%.
Report is 5 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1795      +/-   ##
==========================================
+ Coverage   46.60%   46.75%   +0.16%     
==========================================
  Files         162      162              
  Lines       10079    10078       -1     
  Branches      928      927       -1     
==========================================
+ Hits         4696     4711      +15     
+ Misses       5073     5059      -14     
+ Partials      310      308       -2     

see 3 files with indirect coverage changes

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

docs/sbt.md Outdated Show resolved Hide resolved
docs/sbt.md Outdated Show resolved Hide resolved
docs/sbt.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
@cfmcgrady cfmcgrady marked this pull request as draft August 9, 2023 05:36
@cfmcgrady
Copy link
Contributor Author

cc @pan3793 @waitinfuture PTAL when you have time

docs/developers/sbt.md Outdated Show resolved Hide resolved
docs/developers/sbt.md Outdated Show resolved Hide resolved
To create a Celeborn distribution like those distributed by the [Celeborn Downloads](https://celeborn.apache.org/download/) page, and that is laid out so as to be runnable, use `./build/make-distribution.sh` in the project root directory.

```
./build/make-distribution.sh --sbt
Copy link
Member

Choose a reason for hiding this comment

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

have we supported it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it's a placeholder now, I intend to file a separate PR to support this feature.

Copy link
Member

Choose a reason for hiding this comment

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

but there is a --mvn </path/mvn>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe --sbt-enabled?

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

docs/developers/sbt.md Outdated Show resolved Hide resolved
docs/developers/sbt.md Outdated Show resolved Hide resolved
@pan3793
Copy link
Member

pan3793 commented Aug 10, 2023

Good job! The article is clear and detailed.

@pan3793
Copy link
Member

pan3793 commented Aug 10, 2023

cc @RexXiong @zhongqiangczq @FMX, I suppose it's friendly enough for new developers to learn the SBT.

@cfmcgrady cfmcgrady marked this pull request as ready for review August 10, 2023 13:49
docs/developers/sbt.md Outdated Show resolved Hide resolved
```
For more about how to run individual tests with sbt, see the [sbt documentation](https://www.scala-sbt.org/1.x/docs/Testing.html) and [JUnit Interface](https://github.com/sbt/junit-interface/#junit-interface).

# Accelerating SBT
Copy link
Member

Choose a reason for hiding this comment

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

too generic, it's network specific

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

after thought, I think it's fine, we can put other content in the future, like how to skip unnecessary checks, run tests in parallel,etc.

docs/developers/sbt.md Outdated Show resolved Hide resolved
docs/developers/sbt.md Outdated Show resolved Hide resolved
docs/developers/sbt.md Outdated Show resolved Hide resolved
@pan3793 pan3793 closed this in 516bdc7 Aug 11, 2023
@pan3793
Copy link
Member

pan3793 commented Aug 11, 2023

Thanks, merged to main

@cfmcgrady cfmcgrady deleted the sbt-docs branch August 11, 2023 04:21
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