Skip to content

MINOR: Various README tweaks#18301

Merged
ijuma merged 2 commits intoapache:trunkfrom
ijuma:readme-improvements
Dec 23, 2024
Merged

MINOR: Various README tweaks#18301
ijuma merged 2 commits intoapache:trunkfrom
ijuma:readme-improvements

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Dec 22, 2024

  1. Fix ambiguous statement where it could be interpreted that scalac is used for the clients module
  2. Remove "in KRaft mode" because that's the only mode
  3. Put IntelliJ first in IDE section and only mention the native gradle integration
  4. Restore instructions for installing all projects to the local Maven repo, this was accidentally removed as part of the Scala 2.12 support removal.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@ijuma ijuma requested a review from chia7712 December 22, 2024 20:52
@github-actions github-actions bot added the triage PRs from the community label Dec 22, 2024
@ijuma ijuma force-pushed the readme-improvements branch from befb965 to 24b5695 Compare December 22, 2024 20:52
@github-actions github-actions bot added docs small Small PRs labels Dec 22, 2024
Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@ijuma thanks for this patch. There are two trivial comments remaining.

Comment thread README.md
When it comes to Eclipse, run:

./gradlew eclipse
./gradlew idea
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe we can remove idea plugin from build.gradle?

https://github.com/apache/kafka/blob/trunk/build.gradle#L35

Copy link
Copy Markdown
Member Author

@ijuma ijuma Dec 23, 2024

Choose a reason for hiding this comment

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

This is a larger change as it would break someone who is using this plugin in their workflow (for whatever reason). I'd prefer to start by removing it from the README (to keep things simple for new contributors) and leaving the actual plugin removal for later. Is that reasonable?

Comment thread README.md Outdated

### Installing all projects to the local Maven repository ###

./gradlew publishToMavenLocal
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we combine this with line #178 into a single command? Maybe ./gradlew -PskipSigning=true publishToMavenLocal is good enough

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. I'll still keep the separate line for the single project case, but will add the -PskipSigning=true to this one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pushed this change.

@github-actions github-actions bot removed the triage PRs from the community label Dec 23, 2024
@mumrah
Copy link
Copy Markdown
Member

mumrah commented Dec 23, 2024

@ijuma #17881 adds a "triage" label to PRs from non-committers. Turns out this also affect committers if their membership visibility in the ASF GitHub org is not public. I added instructions for setting your membership visibility to public https://github.com/apache/kafka/blob/trunk/.github/workflows/README.md#pr-triage

The label was removed from this PR since it was reviewed, but if you want to avoid it being added to your future PRs see the link above

@ijuma ijuma merged commit 6fc5c64 into apache:trunk Dec 23, 2024
@ijuma ijuma deleted the readme-improvements branch December 23, 2024 14:58
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Dec 26, 2024

@mumrah Thanks for the heads up, I have changed the visibility to public as documented.

tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
1. Fix ambiguous statement where it could be interpreted that scalac is used for the clients module
2. Remove "in KRaft mode" because that's the only mode
3. Put IntelliJ first in IDE section and only mention the native gradle integration
4. Restore instructions for installing all projects to the local Maven repo, this was accidentally removed as part of the Scala 2.12 support removal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs small Small PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants