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

vokoscreen-beta: init at 2.5.8 #61053

Closed
wants to merge 1 commit into from
Closed

Conversation

GRBurst
Copy link
Contributor

@GRBurst GRBurst commented May 6, 2019

Motivation for this change

Only the stable version of vokoscreen is available, but is missing multiple features that I needed.
I already build and installed it locally and used it multiple times successfully :-)
I guess it could be bundled with the stable version, but this may be more reasonable when the beta and stable version will be maintained continuously.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jtojnar
Copy link
Contributor

jtojnar commented May 6, 2019

The stable version is ancient, I would just update the existing package.

Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Just a couple nitpicks I noticed

pkgs/applications/video/vokoscreen-beta/default.nix Outdated Show resolved Hide resolved
pkgs/applications/video/vokoscreen-beta/default.nix Outdated Show resolved Hide resolved
@GRBurst
Copy link
Contributor Author

GRBurst commented May 7, 2019

@jtojnar I am not sure whether this makes sense to replace a stable version with a beta. I even made sure to replace the target binary to be "vokoscreen-beta" in order to enable having both versions, stable and beta, installed. I agree that the stable is ancient and I can tell that I successfully used the beta a few times now, but from a conceptional point of view I would still disagree 😅
or do you think having both is problematic somehow?

@jtojnar
Copy link
Contributor

jtojnar commented May 7, 2019

A beta package increases the maintenance costs, especially keeping the two expressions in sync.

Is there any reason why someone would need the old version (e.g. removed functionality)? In practice, we often see that while a release was not yet labelled stable, it is actually more stable than the most recent stable build, thanks to newly added bug fixes. For that reason, many packages in nixpkgs often use “unstable” releases or even VCS snapshots.

But really, it needs to be judged on case by case basis. If the beta introduces more bugs than it fixes then parallel expression would be justified. In that case both expressions should share a directory and perhaps inherit from single common expression.

@GRBurst
Copy link
Contributor Author

GRBurst commented May 7, 2019

@jtojnar well okay, I will make a new pr that upgrades the stable to the beta 😉

@worldofpeace worldofpeace changed the title init: vokoscreen-beta at 2.5.8 vokoscreen-beta: init at 2.5.8 May 7, 2019
@GRBurst GRBurst closed this May 15, 2019
@GRBurst GRBurst mentioned this pull request May 28, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants