PaulCombal / SamRewritten

Steam Achievement Manager For Linux. Rewritten in C++.
GNU General Public License v3.0
340 stars 32 forks source link

Cleanup ideas #25

Closed telans closed 4 years ago

telans commented 4 years ago

For cleanup - dev branch

Just thoughts to make the folders look a tad cleaner, will make a PR if wanted

wgpierce commented 4 years ago

I think it makes sense to move the steam headers into src/steam/

They're not source code we wrote for the project, so I think they're fine there. I'm otherwise ambivalent.

Are the libraries inside steam/lib needed? They don't seem to be used for building or running correct, not needed

That's correct, they can be deleted.

I'm ambivalent on the GUI folder change, I think it's fine.

Rename assets/samrewritten64 to icon_64 Rename assets/icon to icon_256 (maybe generate the other common sizes if wanted or when needed)

Not sure why these were added to take things out of sync between this repo and the AUR repo :( The Icon=samrewritten line is also different between the two -- whatever is cleanest to bring these back into sync would be good.

telans commented 4 years ago

Not sure why these were added to take things out of sync between this repo and the AUR repo :( The Icon=samrewritten line is also different between the two -- whatever is cleanest to bring these back into sync would be good.

I think this change, with the one in the PKGBUILD below makes this icon globally available rather than stuck in the /usr/lib/sr directory. Also enables different applications to use the different sizes if available.

install -Dm644 "${_pkgname}/assets/samrewritten64.png" "${pkgdir}/usr/share/icons/hicolor/64x64/apps/samrewritten.png"

I'd rather keep it all contained into one directory but if that's where icons are supposed to end up I'll probably change it on the aur side too. Yeah when they get renamed/merged I'll mirror this on the aur

I would still rename them here to icon_64,256

wgpierce commented 4 years ago

You're right, that's probably the right way to do it. I'll accept pull requests for steam/lib deletion and icon cleanups. I request review of this issue from @PaulCombal for the others since I'm ambivalent.

telans commented 4 years ago

I'll also add onto this,

Move the g++ / make obj directory from the root dir to inside src or package. Or remove them after build

wgpierce commented 4 years ago

Again I'm ambivalent, so I'll leave it up to Paul. There is a clean verb in the makefile which removes them. Keeping the object files around is useful for incremental builds.

PaulCombal commented 4 years ago

I'll also add onto this,

Move the g++ / make obj directory from the root dir to inside src or package. Or remove them after build

It all comes down to personnal preference on this one. Personnally I do not think compiled files have their place in a src folder, and I don't want them to show up in my editor. Though if you're a C++ veteran and think this is a good idea, I'm open to your arguments. Also, it's better not to remove them after a build, like @wgpierce said.

telans commented 4 years ago

Though if you're a C++ veteran and think this is a good idea

Ha!

PaulCombal commented 4 years ago

Please make a new thread for comments for the next release!