Skip to content

[swig] switch to SWIG_ADD_LIBRARY() in CMakeLists.txt (fixes #5586) - #5603

Merged
jameslamb merged 3 commits into
lightgbm-org:masterfrom
shogohida:switch-to-swig-add-library
Nov 27, 2022
Merged

jameslamb merged 3 commits into
lightgbm-org:masterfrom
shogohida:switch-to-swig-add-library

Conversation

@shogohida

Copy link
Copy Markdown
Contributor

Signed-off-by: Shogo Hida shogo.hida@gmail.com

Fixes #5586

Signed-off-by: Shogo Hida <shogo.hida@gmail.com>
@shogohida
shogohida marked this pull request as ready for review November 24, 2022 19:30
@jameslamb jameslamb changed the title fix: Fix CMakeLists.txt [swig] switch to SWIG_ADD_LIBRARY() in CMakeLists.txt Nov 25, 2022
@jameslamb jameslamb changed the title [swig] switch to SWIG_ADD_LIBRARY() in CMakeLists.txt [swig] switch to SWIG_ADD_LIBRARY() in CMakeLists.txt (fixes #5586) Nov 25, 2022

@jameslamb jameslamb left a comment •

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.

Thanks for this! I've modified the PR title so that it's more informative. PR titles become changelog items in this project's release notes (see https://github.com/microsoft/LightGBM/releases/tag/v3.3.3 for example), so we try be more specific than "fix CMakeLists.txt".

Please also fix the failing CI jobs, e.g. this one: https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=13909&view=logs&j=535e9539-0564-5d34-2431-da2644b83c50&t=7cfc64df-bff4-5eef-5deb-8ea58a924da8.

CMake Error at /usr/local/share/cmake-3.23/Modules/UseSWIG.cmake:752 (message):
SWIG_ADD_LIBRARY: java;swig/lightgbmlib.i: unexpected arguments
Call Stack (most recent call first):
CMakeLists.txt:460 (swig_add_library)

Comment thread CMakeLists.txt
Signed-off-by: Shogo Hida <shogo.hida@gmail.com>
Signed-off-by: Shogo Hida <shogo.hida@gmail.com>
@shogohida
shogohida requested review from jameslamb and removed request for StrikerRUS, guolinke and shiyu1994 November 26, 2022 04:42
@shogohida

Copy link
Copy Markdown
Contributor Author

@jameslamb
I fixed the points you mentioned so please review again!

@jameslamb jameslamb left a comment

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.

Looks great, thanks very much for helping to resolve this deprecation warning so new releases of CMake won't disrupt LightGBM's JVM users. We hope you'll consider more contributions in the future.

@jameslamb
jameslamb merged commit 8f9133b into lightgbm-org:master Nov 27, 2022
@shogohida
shogohida deleted the switch-to-swig-add-library branch November 27, 2022 03:44
@shogohida

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing my PR! Hope to contribute more in the future!

@github-actions

Copy link
Copy Markdown
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[swig] switch to SWIG_ADD_LIBRARY() in CMakeLists.txt

2 participants