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

[docs] Update Quick Start #5900

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Conversation

jaceklaskowski
Copy link
Contributor

Make lightgbm command executable on command line

Make `lightgbm` command executable on command line
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in LightGBM abd for taking the time to contribute.

Can you please explain how this change "makes lightgbm executable"? Did you experience some error following the instructions as currently written? What was it?

As a general rule, I think you'll find your proposed changes here (and in other open source projects) are more likely to be accepted if you explain how they improve the project. Without such explanation, you're asking reviewers to guess, which increases the effort required to review.

@jameslamb jameslamb changed the title [DOCS] Update Quick Start [docs] Update Quick Start May 30, 2023
@jaceklaskowski
Copy link
Contributor Author

This term "executable" is used figuratively. Sorry about that. It's just that the Installation Guide allows to install lightgbm binary in any location and since I'm on macOS (and installed it using brew and on the PATH) I found this requirement to have it in the root directory of this project too rigid. Kinda surprising given the guide is for users not developers / maintainers of this project who could have the binary installed in the project root.

Using double quotes made it even more wishing for a change.

All in all, I made the simple changes for two reasons:

  1. It's the very first open-source project where quotes are used to execute a binary on command line (that was unacceptable for me 😉)
  2. I wanted to check out the process and how it really works here

HTH ❤️

@jameslamb jameslamb self-requested a review May 30, 2023 13:57
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Ahhhh ok! Very good point that these docs would be incompatible with other methods like brew install for the CLI.

Thanks for that explanation. Given that, I totally support this change.

Don't worry about the failing CI jobs... they are not a result of your changes. See #5898 and #5899 for more details. Once we're able to fix those (and this project's CI), I'll come back and merge this. Sorry for the inconvenience.

@jameslamb
Copy link
Collaborator

I've updated this branch with the latest changes from master, and will merge it when it builds.

@jameslamb jameslamb merged commit ad487fe into microsoft:master Jun 1, 2023
@github-actions
Copy link

github-actions bot commented Sep 6, 2023

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants