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

Codechange: Drop libxdg-basedir dependency in favour of finding the directories ourselves #8357

Merged
merged 1 commit into from Jan 2, 2021

Conversation

LordAro
Copy link
Member

@LordAro LordAro commented Dec 6, 2020

The dependency is a bit of a pain, and iirc @matthijskooijman expressed interest in dropping the package from debian entirely anyway.

And given replacing the dependency takes so little effort anyway...

Ideally the whole Determine(Base)Path stuff would be replaced with something that didn't rely on a load of manual string manipulation, but that's for later...

@LordAro LordAro force-pushed the remove-xdg-basedir-dep branch 4 times, most recently from 29e8d2e to 8deaf5a Compare December 6, 2020 19:24
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@matthijskooijman matthijskooijman left a comment

Choose a reason for hiding this comment

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

The dependency is a bit of a pain, and iirc @matthijskooijman expressed interest in dropping the package from debian entirely anyway.

I don't really recall this and could not find any evidence, but looking in Ubuntu, there's only a handful packages left using libxdg-basedir, so I guess it's on its way to removal anyway, so +1.

src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
@TrueBrain
Copy link
Member

Something to consider:

WITH_PERSONAL_PATH is with CMake always set. That is to say, unless someone is really out to unset it, but all OSes have a default. So if we take that into account, the USE_XDG macro becomes easier, and I am sure some other parts of the code becomes easier to read too.

No need to do this in this PR. It just jumped out for me :)

@TrueBrain TrueBrain added candidate: yes This Pull Request is a candidate for being merged OS: Linux Issues specific to Linux builds size: trivial This Pull Request is trivial size: small This Pull Request is small, and should be relative easy to process and removed size: trivial This Pull Request is trivial labels Dec 14, 2020
@TrueBrain TrueBrain added the work: minor details This Pull Request has some minor details left to do label Dec 21, 2020
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
@LordAro LordAro force-pushed the remove-xdg-basedir-dep branch 2 times, most recently from c5b7cfe to 29b4b75 Compare January 2, 2021 17:01
src/fileio.cpp Outdated Show resolved Hide resolved
src/fileio.cpp Outdated Show resolved Hide resolved
Copy link
Member

@TrueBrain TrueBrain left a comment

Choose a reason for hiding this comment

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

Couldn't find anything else to bitch about :(

@LordAro LordAro merged commit 3dfee97 into OpenTTD:master Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged OS: Linux Issues specific to Linux builds size: small This Pull Request is small, and should be relative easy to process work: minor details This Pull Request has some minor details left to do
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants