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

remove hardcoded "sgpp/" folder from library install path #222

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

leiterrl
Copy link
Member

This fixes #129 and makes life easier when using packaging software.

@spc90
Copy link
Member

spc90 commented Mar 23, 2020

This doesn't seem to add too much headache to me in the process of making the DM pipeline split work, so I'm fine with it :)
@leiterrl So basically the new location of the .so files in directly in the lib/ folder, not in lib/sgpp/, right? This seems to be it...

@leiterrl
Copy link
Member Author

This doesn't seem to add too much headache to me in the process of making the DM pipeline split work, so I'm fine with it :)
@leiterrl So basically the new location of the .so files in directly in the lib/ folder, not in lib/sgpp/, right? This seems to be it...

Yes that's basically it. This seems to be the expected behavior by other people/software. Also it makes sense to not have hardcoded paths in your build process. The old behavior can always be replicated by setting the appropriate build variables.

@leiterrl leiterrl requested a review from G-071 March 23, 2020 14:27
Copy link
Member

@G-071 G-071 left a comment

Choose a reason for hiding this comment

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

LGTM

@valentjn
Copy link
Member

valentjn commented Mar 23, 2020 via email

@valentjn
Copy link
Member

valentjn commented Mar 23, 2020 via email

@leiterrl
Copy link
Member Author

I think this misses a few occurrences. I recommend grepping for lib.*sgpp (case-insensitive) to find most of them.

Running "grep --exclude-dir=.vscode* --exclude=*.log -InHr lib.*sgpp ." at root did not yield any more useful results for me (asides from finding build scripts by Malte and Fabian that shouldn't be there in the first place).

Regarding the wiki: Yes that has to be skimmed as well. We will also have to send out an email notifying users of the new behavior.

@valentjn
Copy link
Member

@leiterrl I stand corrected, I thought there would be more places.

@leiterrl leiterrl merged commit 6bc5012 into master Mar 23, 2020
@leiterrl leiterrl deleted the fix_install_paths branch March 23, 2020 18:25
@leiterrl leiterrl added this to the v3.3.0 milestone Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

installation layout
4 participants