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

app-editors/scite: Version Bump to 4.2.1. #13464

Closed

Conversation

dermoench42
Copy link
Contributor

Actual enhancements and upstream bugfixes.

Signed-off-by: Ervin Peters ervin.peters@ervnet.de
Package-Manager: Portage-2.3.76, Repoman-2.3.16

Actual enhancements and upstream bugfixes.

Signed-off-by: Ervin Peters ervin.peters@ervnet.de
Package-Manager: Portage-2.3.76, Repoman-2.3.16
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @dermoench42
Areas affected: ebuilds
Packages affected: app-editors/scite

app-editors/scite: ervin.peters[at]ervnet.de, @gentoo/proxy-maint

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.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.


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 assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. labels Oct 27, 2019
}

pkg_postinst() {
gnome2_icon_cache_update
Copy link
Member

Choose a reason for hiding this comment

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

With EAPI-7 this is xdg_icon_cache_update, the gnome- one doesnt work anymore. Same for one below.

}

src_install() {
emake DESTDIR="${ED}" install
Copy link
Member

Choose a reason for hiding this comment

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

Replace with default, You could add a global DOCS=( ... ) array to install that README file below when calling default here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not clear to me. The default implementation is:
src_install() {
if [[ -f Makefile ]] || [[ -f GNUmakefile ]] || [[ -f makefile ]] ; then
emake DESTDIR="${D}" install
fi
einstalldocs
}
which differs in DESTDIR Parameter.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, sure sure, it doesnt matter that much :)

Copy link
Contributor

@xdch47 xdch47 Oct 31, 2019

Choose a reason for hiding this comment

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

however, it does not work on a prefixed enviroment.

scite/gtk/makefile:32 gnomeprefix:=$(shell pkg-config --variable=prefix $(GTKVERSION) 2>/dev/null)

will lead to double prefixing in the end. ( returns $EPREFIX/usr )

one option would be the default emake (DESTDIR="${D}")
another option would be emake DESTDIR="$D" prefix="$EPREFIX/usr"
(also needed for the compilation) (link errors on src_compile).

gnome2_icon_cache_update -> xdg_icon_cache_update

Signed-off-by: Ervin Peters ervin.peters@ervnet.de
Package-Manager: Portage-2.3.76, Repoman-2.3.16
}

src_install() {
emake DESTDIR="${ED}" install
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, sure sure, it doesnt matter that much :)

die "Sorry, SCiTE uses C++17 Features and needs >sys-devel/clang-5
($(clang-major-version))."

elif tc-is-gcc; then
Copy link
Member

Choose a reason for hiding this comment

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

One tab too many.

x11-libs/gdk-pixbuf
x11-libs/gtk+:3=
x11-libs/pango
lua? ( >=dev-lang/lua-5:= )
Copy link
Member

Choose a reason for hiding this comment

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

Lost indentation here.


EAPI=7

inherit gnome2-utils toolchain-funcs xdg-utils
Copy link
Member

Choose a reason for hiding this comment

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

gnome2-utils isnt used anymore, can be dropped. Few styling things, then it looks good to me!

Signed-off-by: Ervin Peters ervin.peters@ervnet.de
Package-Manager: Portage-2.3.76, Repoman-2.3.16
Signed-off-by: Ervin Peters ervin.peters@ervnet.de
Package-Manager: Portage-2.3.76, Repoman-2.3.16
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2019-10-31 02:58 UTC
Newest commit scanned: 3f5aca5
Status: ✅ good

No issues found

if ! use lua; then
emake_pars+=" NO_LUA=1"
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

why not an array?

     # prepare make options
     local emake_pars=("GTK3=1")

     tc-is-clang && emake_pars+=("CLANG=1")
     use !lua    && emake_pars+=("NO_LUA=1")
 
     emake -C "${WORKDIR}/scintilla/gtk" "${emake_pars[@]}"
     emake "${emake_pars[@]}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why? kept from previous versions ;). But an array is more clean, sure. I'll change it.

Copy link
Member

Choose a reason for hiding this comment

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

I was waiting for this, but guess you will fix it in the next version bump? :)

Lets not keep this any longer then, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... scusi... lack of time. Since this was mainly cosmetic i wanted to fix the prefix thing, too. It will be done probably on the next release.

@xdch47
Copy link
Contributor

xdch47 commented Oct 31, 2019

don't forget to rebase/squash commits (+force push)

@gentoo-bot gentoo-bot closed this in 8f9f387 Nov 9, 2019
@xdch47
Copy link
Contributor

xdch47 commented Nov 9, 2019

looks like you commited the version

src_install() {
	emake DESTDIR="${ED}" install

which will lead do double prefix ! - the DOCS=(...) & default would have been better (see above)

so, hopefully next bump...

@juippis
Copy link
Member

juippis commented Nov 11, 2019

@xdch47 your comments were good, thanks. Didn't want to ignore them, but looked like nothing was happening here so I didn't dare to look away any longer. This PR would've been left to rot for months then ;)

But yes, something to remember & fix for the next time...

@dermoench42 dermoench42 deleted the app-editors/scite-4.2.1 branch February 1, 2020 08:13
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). no bug found No Bug/Closes found in the commits.
Projects
None yet
5 participants