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

Rename keepassx2-http to keepassx-reboot #19638

Merged
merged 1 commit into from
Oct 19, 2016

Conversation

jonafato
Copy link
Contributor

Motivation for this change

The keepassx2-http fork has been moved to a new organization and
renamed to keepassx-reboot. For more details on the change, see the
discussions in GitHub issues [1][2].

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Included changes:

  • Rename the keepassx2-http package to keepassx-reboot
  • Fetch source from correct (moved) GitHub repository
  • Update the version to the latest release
  • Change the homepage, as these projects are likely to diverge over
    time
  • Add keepassx2-http to `aliases.nix

[1] keepassx/keepassx#111 (comment)
[2] keepassxreboot/keepassxc#40

@mention-bot
Copy link

@jonafato, thanks for your PR! By analyzing the history of the files in this pull request, we identified @s1lvester, @nbp and @aske to be potential reviewers.

name = "keepassx2-http-unstable-${version}";
version = "2016-05-27";
name = "keepassx-reboot-${version}";
version = "2.0.3-http";
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi. Last time it was pointed out to me, that we shouldn't use these namespaces (I assume you're coming from arch too 😉 ) See. #17778 (diff) by @RamKromberg

I suggest we omit the ´-http´ since the packagename is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2.0.3-http is the name of the tag being fetched from GitHub. If the version name is going to be a problem, I can decouple the version from the tag name. If it's not an issue, it's probably good to remain consistent with the source repository.

Let me know how I should proceed here.

Copy link
Contributor

@RamKromberg RamKromberg Oct 18, 2016

Choose a reason for hiding this comment

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

@s1lvester @jonafato actually things seem to be fine now as far as the parser is concerned:

$ nix-env -iA nixos.nix-repl

$ nix-repl
Welcome to Nix version 1.11.4. Type :? for help.

nix-repl> builtins.parseDrvName "keepassx-reboot-2.0.3-http"
{ name = "keepassx-reboot"; version = "2.0.3-http"; }

nix-repl> builtins.compareVersions "2.0.3-http" "2.0.3-http"
0

nix-repl> builtins.compareVersions "2.0.3-http" "2.0.4-http"
-1

nix-repl> builtins.compareVersions "2.0.4-http" "2.0.3-http"
1

Not sure about the conventions though...

p.s. There still problems when transitioning between formats:

nix-repl> nix-repl> builtins.compareVersions "2.0.4-http" "2.0.4"
1

nix-repl> builtins.compareVersions "2.0.4" "2.0.4-http"
-1

nix-repl> builtins.compareVersions "2.0.4" "2.0.6-http"
-1

nix-repl> builtins.compareVersions "2.0.4-http" "2.0.6"
-1

So those probably keep the conventions as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RamKromberg I've updated the commit to just use 2.0.3 as the version and ${version}-http for the current tag. This should account for new tags that will likely drop the -http suffix.

The `keepassx2-http` fork has been moved to a new organization and
renamed to `keepassx-reboot`. For more details on the change, see the
discussions in GitHub issues [1][2].

Included changes:
- Rename the `keepassx2-http` package to `keepassx-reboot`
- Fetch source from correct (moved) GitHub repository
- Update the version to the latest release
- Change the `homepage`, as these projects are likely to diverge over
  time
- Add `keepassx2-http` to `aliases.nix

[1] keepassx/keepassx#111 (comment)
[2] keepassxreboot/keepassxc#40
@jonafato jonafato force-pushed the init-keepassx-reboot-2.0.3-http branch from 263f6ba to 0bc1865 Compare October 19, 2016 01:00
@s1lvester
Copy link
Contributor

Ping @globin Can you merge this?

@pSub pSub merged commit 80224ed into NixOS:master Oct 19, 2016
@jonafato jonafato deleted the init-keepassx-reboot-2.0.3-http branch October 19, 2016 14:01
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.

5 participants