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

Added installation instructions for Fedora #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PavelStishenko
Copy link

No description provided.

@qsuscs
Copy link
Contributor

qsuscs commented Apr 4, 2024

Thank you for your contribution. However, I have packaged the dependencies in my copr since a few months ago already, and as of yesterday, the editor itself as well. What do you think about including instructions on how to use those instead?


See also [Tui Widgets](https://tuiwidgets.namepad.de/)
and [Termpaint](https://termpaint.namepad.de/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These links should stay here. If we want to avoid repeating them they should be removed from the fedora specific parts.


```
git clone https://github.com/istoph/editor
cd editor
meson setup _build
PKG_CONFIG_PATH=$HOME/opt/tuiwidgets-prefix/lib/x86_64-linux-gnu/pkgconfig meson setup _build -Dprefix=$HOME/opt/tuiwidgets-prefix -Dsyntax_highlighting=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

While these paths match the build / install example from tuiwidgets, i don't think we should assume here that the user has followed these instructions. So these paths come somewhat out of the blue here. Setting prefix to tuiwidgets-prefix doesn't seem like the best suggestion (it's ok to do that when one understands the tradeoffs but it is not what i would recommend for someone just copy and pasting).

Maybe this whole change should instead be a sentence explaining that setting these parameters is needed in certain situations.

sudo dnf install gcc gcc-c++ meson ninja-build pkg-config qt5-qtbase-devel qt5-linguist
```

Fedora do not have packages for [Tui Widgets](https://tuiwidgets.namepad.de/) and [Termpaint](https://termpaint.namepad.de/) so you have to install them from sources.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I hope this is going to be outdated soon.
Maybe this would be better phrased as a condition ("if your distribution does not have ... in a recent enough version ..." style).

@textshell
Copy link
Collaborator

I have packaged the dependencies in my copr since a few months ago already, and as of yesterday, the editor itself as well. What do you think about including instructions on how to use those instead?

I think building from source is likely a use case too, but using the copr repo is a nice additional option that we could mention.

Copy link
Owner

@istoph istoph left a comment

Choose a reason for hiding this comment

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

Great that you have done the work to describe this for Fedora as well.

@@ -27,20 +27,29 @@ Dependencies in Debian trixie:
build-essential meson ninja-build pkg-config qt5-qmake qttools5-dev-tools qtbase5-dev libtermpaint-dev libtuiwidgets-dev libposixsignalmanager-dev
```

Installing dependencies in Fedora:
```
sudo dnf install gcc gcc-c++ meson ninja-build pkg-config qt5-qtbase-devel qt5-linguist
Copy link
Owner

Choose a reason for hiding this comment

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

just like Debian, please only list the packages not how it can be installed.

In my test ci (Fedora33 - 38) I install the following:

dnf -y groupinstall "Development Tools" && \
dnf -y install git devscripts ninja-build pkgconfig qt5-qtbase-devel which python-setuptools python3-testresources chrpath

I assume that qt5-linguist is not needed?

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

Successfully merging this pull request may close these issues.

4 participants