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.

Sign up for GitHub

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

Jump to bottom

www-apps/miniflux: new package #25048

Closed
wants to merge 2 commits into from
Closed

www-apps/miniflux: new package #25048

wants to merge 2 commits into from

Conversation

0xC0ncord
Copy link
Member

Add www-apps/miniflux and acct-user/miniflux to the tree.

cc @perfinion @juippis

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @0xC0ncord
Areas affected: ebuilds
Packages affected: acct-user/miniflux, www-apps/miniflux

acct-user/miniflux: @gentoo/proxy-maint (new package)
www-apps/miniflux: @gentoo/proxy-maint (new package)

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

New packages

This Pull Request appears to be introducing new packages only. Due to limited manpower, adding new packages is considered low priority. This does not mean that your Pull Request will not receive any attention, however, it might take quite some time for it to be reviewed. In the meantime, your new ebuild might find a home in the GURU project repository: the ebuild repository maintained collaboratively by Gentoo users. GURU offers your ebuild a place to be reviewed and improved by other Gentoo users, while making it easy for Gentoo users to install it and enjoy the software it adds.

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). labels Apr 16, 2022
@perfinion perfinion self-assigned this Apr 16, 2022
@juippis juippis added the do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. label Apr 17, 2022
perfinion
perfinion suggested changes Apr 17, 2022
View reviewed changes
Copy link
Member

@perfinion perfinion left a comment

Choose a reason for hiding this comment

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

Pretty good overall, just a few small comments :)

acct-user/miniflux/miniflux-0.ebuild Outdated

DESCRIPTION="User for www-apps/miniflux"
ACCT_USER_ID=404
ACCT_USER_HOME="/dev/null"
Copy link
Member

Choose a reason for hiding this comment

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

Nobody and a few other packages use /var/empty, one package uses /dev/null but /var/empty seems better.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't simply not setting the variable do the right thing?

acct-user/miniflux/miniflux-0.ebuild Outdated Show resolved Hide resolved
acct-user/miniflux/miniflux-0.ebuild
DESCRIPTION="User for www-apps/miniflux"
ACCT_USER_ID=404
ACCT_USER_HOME="/dev/null"
ACCT_USER_GROUPS=( nobody )
Copy link
Member

Choose a reason for hiding this comment

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

The group seems inconsistent across the different files.

  • This is nobody.
  • miniflux.initd has nogroup
  • miniflux.service has nobody

I'm not sure which is the right one, but should be the same

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. Yeah this is supposed to be nobody everywhere. I just missed one entry in the init script it seems. Easy to fix.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't a comment on whether what you're doing here is right or wrong, but wanted to share these anyway as they're at the very least informative but also relevant to your interests:

The latter two are less-related but still interesting and kind of in the same vein.

www-apps/miniflux/miniflux-2.0.36.ebuild
local DOCS=(
ChangeLog
README.md
"${FILESDIR}"/README.gentoo
Copy link
Member

Choose a reason for hiding this comment

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

https://devmanual.gentoo.org/eclass-reference/readme.gentoo-r1.eclass/index.html
maybe use this eclass instead? It takes care of saving the file and also printing it only during the first install so maybe can replace most of your postinst()

Copy link
Member Author

Choose a reason for hiding this comment

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

The included readme is pretty big, so I decided against using the readme.gentoo eclass to display it. I still want to show information on first install, so I would like to at least hint to users what they need to do. However, to use the eclass here would mean I would need 2 readme files which doesn't seem like the right way.

www-apps/miniflux/files/miniflux.initd Outdated Show resolved Hide resolved
www-apps/miniflux/miniflux-2.0.36.ebuild Outdated Show resolved Hide resolved
@0xC0ncord 0xC0ncord force-pushed the master branch 2 times, most recently from d17ec2b to c304c3a Compare April 17, 2022 23:18
@0xC0ncord
acct-user/miniflux: new package, add 0
6cd0694 
Signed-off-by: Kenton Groombridge <concord@gentoo.org>
@0xC0ncord 0xC0ncord force-pushed the master branch 2 times, most recently from 77a6b4d to 5bf8c42 Compare May 6, 2022 17:42
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-05-06 17:47 UTC
Newest commit scanned: 77a6b4d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/14ed0e6072/output.html

@0xC0ncord
www-apps/miniflux: new package, add 2.0.36
4a7fe60 
Signed-off-by: Kenton Groombridge <concord@gentoo.org>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2022-05-06 18:02 UTC
Newest commit scanned: 4a7fe60
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/1134f984a2/output.html

@gentoo-bot gentoo-bot closed this in 56b29be May 6, 2022
matoro added a commit to matoro/overlay that referenced this pull request May 31, 2022
@matoro
www-apps/miniflux: remove, in ::gentoo
843c234 
gentoo/gentoo#25048
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@perfinion perfinion perfinion requested changes

@Flowdalic Flowdalic Flowdalic left review comments

@thesamesam thesamesam thesamesam left review comments

@juippis juippis juippis approved these changes

Assignees

@perfinion perfinion

Labels
assigned PR successfully assigned to the package maintainer(s). do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
Milestone
No milestone
7 participants
@0xC0ncord @gentoo-bot @gentoo-repo-qa-bot @perfinion @Flowdalic @thesamesam @juippis