Skip to content

remove support for Solaris (fixes #5216)#5226

Merged
guolinke merged 2 commits intomasterfrom
remove-solaris
May 22, 2022
Merged

remove support for Solaris (fixes #5216)#5226
guolinke merged 2 commits intomasterfrom
remove-solaris

Conversation

@jameslamb
Copy link
Collaborator

Fixes #5216 .
Related to #3513.

Per the discussion in #5217 (comment), this PR proposes removing support for the Solaris operating system in LightGBM.

That support was added only to get {lightgbm} (the R package) onto CRAN, so that it would be easier for R users to install.
There is strong evidence (provided in that comment) that CRAN no longer requires packages to be installable on Solaris.

I'm proposing removing this support for the following reasons:

  1. R Hub is the only easy-to-use option we have found for testing that LightGBM is portable to Solaris, and the fact that CRAN no longer requires such portability means that incidents like [ci] Solaris R hub build is failing: "installation of package ‘Matrix’ had non-zero exit status" #5216 which prevent us from testing on R Hub could easily happen again
  2. LightGBM is already struggling from a lack of maintainer activity, and removing a source of maintenance burden will allow us to focus our limited time and attention on higher-value areas of the project.

Copy link
Collaborator

@guolinke guolinke left a comment

Choose a reason for hiding this comment

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

I like the PR that deletes codes 😆

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Excellent, thanks a lot! Really like to see these changes!

testthat::skip_if(
ON_SOLARIS || ON_32_BIT_WINDOWS
ON_32_BIT_WINDOWS
, message = "Skipping on Solaris and 32-bit Windows"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
, message = "Skipping on Solaris and 32-bit Windows"
, message = "Skipping on 32-bit Windows"

@guolinke guolinke merged commit b077415 into master May 22, 2022
@guolinke guolinke deleted the remove-solaris branch May 22, 2022 01:28
@jameslamb
Copy link
Collaborator Author

Thanks for merging this @guolinke , but I didn't get a chance to address the suggestion in #5226 (comment). I'll open a follow-up PR to address that.

In the future, can you please avoid merging before all suggestions have been addressed?

@guolinke
Copy link
Collaborator

Thanks for merging this @guolinke , but I didn't get a chance to address the suggestion in #5226 (comment). I'll open a follow-up PR to address that.

In the future, can you please avoid merging before all suggestions have been addressed?

Sorry, I just saw 2 approves, will check the details next time.

@jameslamb
Copy link
Collaborator Author

no problem! @StrikerRUS, @jmoralez and I sometimes approve but with minor suggestions, meaning "I approve assuming you fix this minor suggestion", to avoid another round of reviews.

@guolinke
Copy link
Collaborator

no problem! @StrikerRUS, @jmoralez and I sometimes approve but with minor suggestions, meaning "I approve assuming you fix this minor suggestion", to avoid another round of reviews.

Actually, I tried the GitHub App on my mobile phone to review issues and codes yesterday, and didn't realize it hides the approved comment by default 🤣 .

@github-actions
Copy link
Contributor

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 Aug 19, 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.

[ci] Solaris R hub build is failing: "installation of package ‘Matrix’ had non-zero exit status"

3 participants