PaulCombal / SamRewritten

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

vscode / script changes #15

Closed telans closed 5 years ago

PaulCombal commented 5 years ago

Looks great! We'll first do a big release, then we will probably merge this. One thing though, can you keep the executable name to lowercase ./bin/samrewritten? It would respect the POSIX conventions (see guideline 2)

In the future I'll look into making an AppImage, I believe the uppercase letters makes sense for this.

Thanks!

telans commented 5 years ago

Will do

wgpierce commented 5 years ago

LGTM. telans, I was going to make a PKGBUILD sometime for arch distros. If you want to make it sometime before I get to it, that'd be cool too. Then we can put it on the AUR if we want to.

telans commented 5 years ago

@wgpierce

There's already an AUR package, but I don't like how the name samrewritten is hyphenated. It also installs files which are not needed.

I did modify that PKGBUILD a bit so only required files are installed. This also uses the beta-ipc branch.

pkgname="samrewritten-git"
_pkgname="SamRewritten"
pkgver=r58.b13cdcf
pkgrel=1
pkgdesc="A Steam Achievement Manager For Linux."
arch=("any")
url="https://github.com/PaulCombal/SamRewritten"
license=("GPL3")
depends=("steam" "yajl" "gtk3" "glibc")
makedepends=("git")
conflicts=("sam-rewritten-git")
source=("git+https://github.com/PaulCombal/SamRewritten.git#branch=beta-ipc")
md5sums=("SKIP")

pkgver() {
    cd ${_pkgname}
    printf "r%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
}

build() {
    cd ${_pkgname}
    ./make.sh
}

package() {  
    install -dm755 "${pkgdir}/usr/lib/"
    cp -r ${_pkgname} "${pkgdir}/usr/lib/"
    install -dm755 "${pkgdir}/usr/bin"
    ln -s "${pkgdir}/usr/lib/${_pkgname}/bin/launch.sh" "${pkgdir}/usr/bin/samrewritten"

    rm -rf ${pkgdir}/usr/lib/${_pkgname}/{.git*,.vscode,bin/test,src,steam/*.*}
}
wgpierce commented 5 years ago

Oh dang I didn't find that package before because of its hyphenation. Don't put out the beta-ipc branch yet until we merge to master -- we're almost there.

@telans your version still includes a bunch of unneeded files:

# short output from pacman -Qlp samrewritten-git-r58.b13cdcf-1-any.pkg.tar
# of files that are not needed:

samrewritten-git /usr/lib/SamRewritten/bin/.gitignore
samrewritten-git /usr/lib/SamRewritten/glade/.gitignore
samrewritten-git /usr/lib/SamRewritten/glade/mockup_achievement_list_row.glade
samrewritten-git /usr/lib/SamRewritten/make.sh
samrewritten-git /usr/lib/SamRewritten/steam/
samrewritten-git /usr/lib/SamRewritten/steam/lib/
samrewritten-git /usr/lib/SamRewritten/steam/lib/linux32/
samrewritten-git /usr/lib/SamRewritten/steam/lib/linux32/libsdkencryptedappticket.so
samrewritten-git /usr/lib/SamRewritten/steam/lib/linux64/
samrewritten-git /usr/lib/SamRewritten/steam/lib/linux64/libsdkencryptedappticket.so

You only really need samrewritten, libsteam_api.so, launch.sh, and main_window.glade, and probably README.md and LICENSE currently, so it'd probably be easier to just manually include all those.

telans commented 5 years ago

Would you happen to know what libsdkencryptedappticket.so is used for?

wgpierce commented 5 years ago

They're not used for anything; they're just artifacts of where Paul gets the steam_api library headers. I just tried compiling and running without them and nothing went wrong, so nothing uses them. They could probably be removed.

nomis6432 commented 5 years ago

@telans I've added you as a maintainer in the AUR since you are a contributor to the project and know how PKGBUILDs work. Feel free to push the changes to remove certain unnecessary files to the AUR package.

Changing the AUR name could be done by creating a new package and requesting the old package to be merged with the new one.

telans commented 5 years ago

I've pushed the aur changes and added a desktop entry, if anyone wants to take a look

wgpierce commented 5 years ago

@telans that looks good now. Any possibility of changing to samrewritten package name? After the new icon is added, we can change the icon on the .desktop file to use the new icon. I think we can wait for Paul to approve this pull now.

nomis6432 commented 5 years ago

@wgpierce to change the package name you'd have to create a new package with the new name and submit a merge request to merge the old package in the new package.

I only created the AUR package because I needed it myself and thought it could be useful to others. If you want to you can create that new package yourself and I'll submit a merge request on my package in your package.

Note that I've added "samrewritten" to the key words so it will already show up when you search for it.

telans commented 5 years ago

Creating a package with the name samrewritten-git now

telans commented 5 years ago

@nomis6432 https://aur.archlinux.org/packages/samrewritten-git/

wgpierce commented 5 years ago

@nomis6432 yeah I figured it would be a little bit of a hassle. Thanks telans. We can wait for Paul to weigh in as well.

telans commented 5 years ago

Added you as a co-maintainer too, @wgpierce

nomis6432 commented 5 years ago

Ok I'll sibmit a merge request. It might take a while since it has to be reviewed by one of the thrusted users.

PaulCombal commented 5 years ago

Sorry I didn't really take an in-depth look, but for now if anything works at all it's ok. We're going to release a new version soon, and I would like the release after that to be a "cleanup operation" where all this stuff should be taken care of in-depth. Then it would be great to include the PKGBUILD in the git repo.

telans commented 5 years ago

Alright, I'll hold off on this and reopen when the new version is released

PaulCombal commented 5 years ago

@wgpierce Should merge beta-ipc into master next time he's online, and I'll start doing some cleanup right after, so stay put!

wgpierce commented 5 years ago

@telans , reopening for you to rebase on master and I'll accept it.

wgpierce commented 5 years ago

oh deleting the branch causes the issue to be closed. @telans if you make a new issue on master, I'll accept it right away!

wgpierce commented 5 years ago

Merged in https://github.com/PaulCombal/SamRewritten/pull/17

Thanks @telans! and thanks for adding me as maintainer, though you've got a good handle on it. And thanks @nomis6432 for the original PKGBUILD!