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

dev-util/hut: version bump to 0.5.0 #191

Closed
wants to merge 1 commit into from

Conversation

cjbayliss
Copy link
Contributor

No description provided.

@cjbayliss
Copy link
Contributor Author

As of yesterday, according to https://git.sr.ht/~emersion/hut, the maintainer and repo for hut has changed owner. It is now here: https://git.sr.ht/~xenrox/hut

Since there are no new releases, I don't think this PR should change.

Copy link
Contributor

@antecrescent antecrescent left a comment

Choose a reason for hiding this comment

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

As of yesterday, according to https://git.sr.ht/~emersion/hut, the maintainer and repo for hut has changed owner. It is now here: https://git.sr.ht/~xenrox/hut

Since there are no new releases, I don't think this PR should change.

Since the upstream information in metadata.xml now needs changing, I suggest you tackle both that and this ebuild at once, so that they are up to date and in sync :)

Changes for metadata.xml:

  • Remove <remote-id type="github">apraga/hut-vendor</remote-id>
    It's not related to upstream.
  • Update <remote-id type="sourcehut">~emersion/hut</remote-id> to reflect the new upstream.

Thank you for bumping this package!

@@ -21,7 +21,6 @@ SLOT="0"

KEYWORDS="~amd64"


DEPEND="${RDEPEND}"
BDEPEND="app-text/scdoc"
Copy link
Contributor

Choose a reason for hiding this comment

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

Manpages that require additional deps to be built, should be shipped alongside the package: https://projects.gentoo.org/qa/policy-guide/installed-files.html#pg0305
Could you do that and drop the BDEPEND, please?

Choose a reason for hiding this comment

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

This is incorrect. The policy states that:

  • USE=man must not be used, since the package must always install a manpage
  • prebuilding it and shipping it as an additional source is a good alternative to requiring "inconvenient" additional dependencies

scdoc isn't an inconvenient additional dependency. I don't think it's necessary to prebuild it, as the package currently doesn't violate the policy document.

Copy link
Contributor

Choose a reason for hiding this comment

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

@eli-schwartz It seems I misunderstood part of the policy. Your second bullet point clarified it for me, thanks!

@@ -21,7 +21,6 @@ SLOT="0"

KEYWORDS="~amd64"


DEPEND="${RDEPEND}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this line, since both RDEPEND and DEPEND are empty.

@@ -11,7 +11,7 @@ HOMEPAGE="https://sr.ht/~emersion/hut/"

SRC_URI="
https://git.sr.ht/~emersion/${PN}/archive/v${PV}.tar.gz -> ${P}.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update this to reflect the upstream change.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Fix the copyright, please.
    The package was introduced to ::guru last year and it should read 2023-2024.
  • Update HOMEPAGE to reflect the upstream change.
  • If you decide to ship a pre-built manpage, remove the doc/hut.1 target in src_compile and install the file in src_install.

also tidy up the ebuild:

* upstream changed, see the notice in the old upstream's README:
  https://git.sr.ht/~emersion/hut
* correct the copyright years
* drop 'DEPEND="${RDEPEND}"'

Signed-off-by: Christopher Bayliss <cjbdev@icloud.com>
@cjbayliss
Copy link
Contributor Author

Thank you for the review!! ❤️

I tried dropping the dependency on scdoc, and ran into an issue where the man page was greater than 20KB. Since I didn't know how to deal with that, I asked on #gentoo-dev-help. They suggested adding the file to SRC_URI instead of the repo.

Then they suggested that depending on scdoc wasn't a bad idea since it isn't hard to install. I agree with them, so I have not dropped scdoc.

If you insist on it, I'll drop scdoc and host the man page in the same repo I'm using the the go modules. 🙂

@MrRoy
Copy link
Contributor

MrRoy commented May 21, 2024

LGTM, merged in 027c1e5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants