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

reorganizing youtube-viewers #4128

Merged
merged 21 commits into from
May 28, 2021
Merged

reorganizing youtube-viewers #4128

merged 21 commits into from
May 28, 2021

Conversation

pirate486743186
Copy link
Contributor

reorganizing youtube-viewer and straw-viewer with youtube-viwers-common
adding pipe-viewer

@rusty-snake rusty-snake marked this pull request as draft March 22, 2021 19:01
Copy link
Collaborator

@rusty-snake rusty-snake left a comment

Choose a reason for hiding this comment

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

  • gtk-pipe-viewer and pipe-viewer need to be added to firecfg.config.
  • The private-bin lines need more work:
    • /usr/bin/firefox is only wrapper script on the most system. Every distro has its own. The current private-bin does not work for firefox on the most system.
    • why do we have no chromium, whatever?
    • Having a private-bin with e.g. vlc give the impression it is supported to start it though I don't think this works because of the whitelist and disable-program.inc.
  • quiet does not work this way (see below).

Something like this should be first done on a branch on your fork. Then you can create a PR with it.
A Draft/WIP PR is good for bigger changed to get earlier review but not to active develop (= commit every few minutes) on it.

etc/profile-m-z/youtube-viewers-common.profile Outdated Show resolved Hide resolved
etc/profile-m-z/straw-viewer.profile Show resolved Hide resolved
etc/profile-m-z/youtube-viewer.profile Outdated Show resolved Hide resolved
etc/profile-m-z/pipe-viewer.profile Show resolved Hide resolved
etc/profile-a-l/gtk-pipe-viewer.profile Outdated Show resolved Hide resolved
@pirate486743186
Copy link
Contributor Author

* [ ]  gtk-pipe-viewer and pipe-viewer need to be added to firecfg.config.

I think you where the one complaining that it creates merge conflicts?

* [ ]  The `private-bin` lines need more work:
  
  * [ ]  /usr/bin/firefox is only wrapper script on the most system. Every distro has its own. The current private-bin does not work for firefox on the most system.
  * [ ]  why do we have no chromium, whatever?
  * [ ]  Having a private-bin with e.g. vlc give the impression it is supported to start it though I don't think this works because of the `whitelist` and disable-program.inc.

profiles for youtube-viewer, straw-viewer and their gtk front ends already existed. I copy pasted what they had.

@pirate486743186
Copy link
Contributor Author

lol... yea, vlc is broken (it wasn't me)

they use a video player and a terminal like addons.
they can also launch a webpage in a browser.
The exact application doesn't matter.

so firefox, mpv, xterm are just the usual defaults that will work for most users.

@rusty-snake
Copy link
Collaborator

gtk-pipe-viewer and pipe-viewer need to be added to firecfg.config.

I think you where the one complaining that it creates merge conflicts?

I said "(for the future) Please don't add new profiles to README.md/RELNOTES in PRs, it just create merge conflicts" (#4023 (comment)). README.md and RELNOTES have everything on a few line and are added by appending. firecfg.config has it over hundreds lines which are ordered alphabetically (more or less). README.md/RELONTES will conflict if two PR with a new profile are create while firecfg only if they have a name with the same prefix (e.g. youtube-viewer and youtube-watcher).

profiles for youtube-viewer, straw-viewer and their gtk front ends already existed. I copy pasted what they had.

I know I just saw that this doesn't make any sense and we should correct this.

so firefox, mpv, xterm are just the usual defaults that will work for most users.

Firefox needs to be tested on Arch, Fedora, Debain and maybe more.

@pirate486743186
Copy link
Contributor Author

the external applications that are launched are configurable. You can literally use any terminal, video player and web browser.

firefox, mpv and xterm are nice defaults.

the video player is very important. The terminal is for a lesser feature, and the web browser is of little importance.

i think it's better to remove vlc. And we shouldn't stress about firefox or chromium.

src/firecfg/firecfg.config Outdated Show resolved Hide resolved
@pirate486743186 pirate486743186 changed the title [WIP]reorganizing youtube-viewers reorganizing youtube-viewers Apr 9, 2021
@pirate486743186
Copy link
Contributor Author

why you aren't merging it?

@rusty-snake
Copy link
Collaborator

  • PR is still in Draft state
  • wruc-for-all thread is unresolved
  • noinput is missing (because of the long time)

@pirate486743186 pirate486743186 marked this pull request as ready for review May 27, 2021 19:03
@rusty-snake rusty-snake merged commit 0fd1534 into netblue30:master May 28, 2021
@rusty-snake
Copy link
Collaborator

merged, thanks!

@pirate486743186
Copy link
Contributor Author

you forgot to mention pipe-viewer and gtk-pipe-viewer in the relnotes....

@rusty-snake
Copy link
Collaborator

I haven't forgotten them, I just don't always add them immediately.

@matu3ba matu3ba mentioned this pull request Oct 7, 2021
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.

3 participants