Add few desktop files, this fixes bug:639 #9

Merged
Ghost merged 1 commits from bug/639 into master 5 years ago
Ghost commented 5 years ago

I came up to this in order to fix bug639.

Everyone will get the same desktop files as well as the same icons.
The desktop files have been taken from your Debian packaging system.

Not sure It will work for the *BSD since they put their files in /usr/local thought.

Tell me what you think about this.

I came up to this in order to fix bug639. Everyone will get the same desktop files as well as the same icons. The desktop files have been taken from your Debian packaging system. Not sure It will work for the *BSD since they put their files in /usr/local thought. Tell me what you think about this.
Ghost added the PR/rfc label 5 years ago
Ghost added the PR/wip label 5 years ago
MicheleC changed title from add few desktop files, this fixes bug:639 to WIP: add few desktop files, this fixes bug:639 5 years ago
Ghost commented 5 years ago
Poster

I've added a sharedir option to configure.

Something to test like:

./configure \
     -prefix "/usr" \
     -sysshare "/usr/share"
I've added a sharedir option to configure. Something to test like: ``` ./configure \ -prefix "/usr" \ -sysshare "/usr/share" ```
SlavekB reviewed 5 years ago
SlavekB left a comment
Owner

There are only small comments. Please consider naming the variable "sysshare" instead of "share" because its purpose is similar to "sysconf" variable.

There are only small comments. Please consider naming the variable "sysshare" instead of "share" because its purpose is similar to "sysconf" variable.
configure Outdated
sysconfdir)
QT_INSTALL_SYSCONF="$VAL"
;;
sharedir)
Owner

It seems that the indentation is different than the conditions that are in the neighborhood.

It seems that the indentation is different than the conditions that are in the neighborhood.
configure Outdated
# default PREFIX/etc/settings
[ -z "$QT_INSTALL_SYSCONF" ] && QT_INSTALL_SYSCONF=$QT_INSTALL_PREFIX/etc/settings
# default PREFIX/usr/share
[ -z "QT_INSTALL_SHARE" ] && QT_INSTALL_SHARE=$$QT_INSTALL_PREFIX/usr/share
Owner

Because the prefix will usually be set to /usr, /usr/local, or something similar, I think the default value for QT_INSTALL_SHARE should not contain /usr.

Because the prefix will usually be set to `/usr`, `/usr/local`, or something similar, I think the default value for `QT_INSTALL_SHARE` should not contain `/usr`.
Owner

@cethyel, please can you simplify the commit log a bit?

For example, it doesn't make much sense to include in one comment "add a sharedir option…" and then "rename share option to sysshare…", 3× the same "Signed-off-by", etc.

@cethyel, please can you simplify the commit log a bit? For example, it doesn't make much sense to include in one comment "add a sharedir option…" and then "rename share option to sysshare…", 3× the same "Signed-off-by", etc.
SlavekB reviewed 5 years ago
SlavekB left a comment
Owner

There is a small typo that requires correction.

There is a small typo that requires correction.
configure Outdated
# default PREFIX/etc/settings
[ -z "$QT_INSTALL_SYSCONF" ] && QT_INSTALL_SYSCONF=$QT_INSTALL_PREFIX/etc/settings
# default PREFIX/share
[ -z "QT_INSTALL_SHARE" ] && QT_INSTALL_SHARE=$QT_INSTALL_PREFIX/share
Owner

There is no $ before the variable name in [ -z "QT_INSTALL_SHARE"].
As a result, the default value is not used.

There is no `$` before the variable name in `[ -z "QT_INSTALL_SHARE"]`. As a result, the default value is not used.
SlavekB approved these changes 5 years ago
SlavekB left a comment
Owner

Tested along with a patch for tde-packaging, there were no problems.

Tested along with a patch for tde-packaging, there were no problems.
SlavekB changed title from WIP: add few desktop files, this fixes bug:639 to Add few desktop files, this fixes bug:639 5 years ago
SlavekB removed the PR/wip PR/rfc labels 5 years ago
Ghost closed this pull request 5 years ago
Ghost deleted branch bug/639 5 years ago
SlavekB added this to the R14.0.7 release milestone 5 years ago
The pull request has been merged as 6131b4262e.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tqt3#9
Loading…
There is no content yet.