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

lxde-base/lxterminal: port to EAPI 7 #17359

Closed
wants to merge 1 commit into from
Closed

Conversation

jsmolic
Copy link
Member

@jsmolic jsmolic commented Sep 1, 2020

Closes: https://bugs.gentoo.org/739838
Package-Manager: Portage-3.0.4, Repoman-3.0.1
Signed-off-by: Jakov Smolic jakov.smolic@sartura.hr

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @jsmolic
Areas affected: ebuilds
Packages affected: lxde-base/lxterminal

lxde-base/lxterminal: @gentoo/proxy-maint (maintainer needed)

Linked bugs

Bugs linked: 739838


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

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added maintainer-needed There is at least one affected package with no maintainer. Review it if you can. assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Sep 1, 2020
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-09-01 11:46 UTC
Newest commit scanned: f5e795f
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/be683e3d73/output.html

# Distributed under the terms of the GNU General Public License v2

EAPI=6
EAPI=7
Copy link
Member

Choose a reason for hiding this comment

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

EAPI bump requires a revbump. And I'm looking at the ebuild and it's awful, so you'd need to fix a few things before it's accepted.

So just inherit xdg and nothing else, or fix the ebuild with a revbump.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I would like to fix it properly then.
Can you tell me what else needs to be changed?

Copy link
Member

Choose a reason for hiding this comment

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

There's no implication that it's installing translation files, I'd put building them behind nls USE flag and well build everything instead of having the PLOCALES array with it. Those files take a second to build and we're already recommending to use INSTALL_MASK for unwanted localization files. This'd allow removing l10n.eclass altogether.

Then it has a case for a -9999 ebuild but no such ebuild exists.

It also has double KEYWORDS line, which breaks some tools.

Also do multi-line the gtk3 USE conditionals for clarity.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I cleaned it up. I didn't find the option to disable building locales with configure options, but I think it's okay to install them unconditionally.

- Adjust DEPEND -> BDEPEND
- Update icon cache
- Remove ${PV} == *9999* logic
- Install locales unconditionally

Closes: https://bugs.gentoo.org/739838
Package-Manager: Portage-3.0.4, Repoman-3.0.1
Signed-off-by: Jakov Smolic <jakov.smolic@sartura.hr>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-09-17 07:12 UTC
Newest commit scanned: 726f2a8
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/0e908d4c5b/output.html

@jsmolic jsmolic deleted the lxterminal branch September 28, 2020 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. maintainer-needed There is at least one affected package with no maintainer. Review it if you can.
Projects
None yet
4 participants