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

New package: soju-0.3.0 #33302

Merged
merged 1 commit into from Nov 21, 2021
Merged

New package: soju-0.3.0 #33302

merged 1 commit into from Nov 21, 2021

Conversation

flupe
Copy link
Contributor

@flupe flupe commented Oct 3, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

Does it build and run successfully?

(Please choose at least one native build and, if supported, at least one cross build. More are better.)

  • I built this PR locally for my native architecture, (ARCH-LIBC)

@ericonr ericonr added the new-package This PR adds a new package label Oct 3, 2021
@Vaelatern
Copy link
Member

srcpkgs/soju/template: Place distfiles= after changelog=
srcpkgs/soju/template:7: short_desc should not start with an article
srcpkgs/soju/template:20: please indent with tabs

@flupe
Copy link
Contributor Author

flupe commented Oct 4, 2021

Thanks, my bad I forgot to run xlint before submitting a PR.
I moved go and scdoc to makehostdepends, however it's unclear to me whether scdoc must be here or in makedepends.

srcpkgs/soju/template Outdated Show resolved Hide resolved
srcpkgs/soju/template Outdated Show resolved Hide resolved
@flupe
Copy link
Contributor Author

flupe commented Oct 4, 2021

Thanks for the review, I made the recommended changes. Uses the go build-style now.
Also removed the soju system user as it looked like more work than necessary.
I think I made the mistake of doing make doc/soju.1 rather than using scdoc in post_install(), but other than that I think it's done.

@Duncaen
Copy link
Member

Duncaen commented Oct 4, 2021

This should use a separate user, there is no reason to run an irc bouncer as root.

system_accounts="_soju"

and exec chpst -u _soju soju ... in the run file.

I also switched the log and database directories, not sure if this would make sense:

make_dirs="/var/db/soju/ 750 _soju _soju
  /var/log/soju/ 750 _soju _soju"

And then chaning the default configuration to use those instead of /var/lib/soju.

@flupe
Copy link
Contributor Author

flupe commented Oct 4, 2021

@Duncaen

This should use a separate user, there is no reason to run an irc bouncer as root.

I completely agree with this statement. My only gripe was that the database is only created when running sojuctl:

sojuctl -config /etc/soju/config create-user username -admin

And users would have to make sure it is ran as _soju so that the service can read it. Is there a way to document this?
Also, is there a naming convention for system users on void, why _soju rather than soju? (Ah, found it on the manual)

I also switched the log and database directories, not sure if this would make sense:

Yes, makes a lot of sense. On it.

@flupe
Copy link
Contributor Author

flupe commented Oct 4, 2021

Did all the recommended changes. Defining a _soju system user, providing an alternative default config using /var/log/soju and /var/db/soju.
Everything seems to work fine now.

Thank you all, sorry for the tedious process, it's my first template. Slowly getting used to the void process.

@Duncaen
Copy link
Member

Duncaen commented Oct 4, 2021

Some packages have a void specific readme, this would be a good case for that. This is done by adding a README.voidlinux into files and installing it with vdoc "${FILESDIR}/README.voidlinux".

@flupe
Copy link
Contributor Author

flupe commented Oct 4, 2021

Added a README.voidlinux.

srcpkgs/soju/template Outdated Show resolved Hide resolved
@Duncaen
Copy link
Member

Duncaen commented Nov 20, 2021

Do you mind updating this to 0.3.0, this has been sitting a while and I don't see anything else blocking a merge.

@flupe flupe force-pushed the master branch 2 times, most recently from 9386ca0 to e2b27e4 Compare November 21, 2021 08:48
@flupe flupe changed the title New package: soju-0.2.1 New package: soju-0.3.0 Nov 21, 2021
@flupe
Copy link
Contributor Author

flupe commented Nov 21, 2021

Done!

@Vaelatern Vaelatern merged commit 92e6bc0 into void-linux:master Nov 21, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-package This PR adds a new package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants