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.

Sign up for GitHub

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

Jump to bottom

zsh-git-prompt: switch to a new fork. #56996

Merged
merged 2 commits into from Mar 8, 2019
Merged

zsh-git-prompt: switch to a new fork. #56996

merged 2 commits into from Mar 8, 2019

Conversation

qolii
Copy link
Contributor

@qolii qolii commented Mar 6, 2019

Motivation for this change

The git repo this package is based on has been left unmaintained for some years. Based on a thread in that repo, a new(-ish) fork seems to have emerged with fixes and ongoing maintainership. I found this while looking for solutions to issues I was seeing with the existing package, and this solved them all for me.

I propose we switch our package to point to this fork instead. I have been using this change for the last week or so and found it generally works well. Wider testing is probably needed(!)

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 nox --run "nox-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.

infinisil
infinisil requested changes Mar 8, 2019
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Other than the following this looks good to me, seems to work fine

pkgs/shells/zsh/zsh-git-prompt/default.nix
sha256 = "19x1gf1r6l7r6i7vhhsgzcbdlnr648jx8j84nk2zv1b8igh205hw";
url = "https://github.com/starcraftman/zsh-git-prompt.git";
rev = "11b83ba3b85d14c66cf2ab79faefab6d838da28e";
sha256 = "04aylsjfb03ckw219plkzpyiq4j9g66bjxa5pa56h1p7df6pjssb";
Copy link
Member

Choose a reason for hiding this comment

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

Can you switch this to fetchFromGitHub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

I also changed the version to reflect the fact that the fork hasn't actually released a 0.5 version yet (see starcraftman/zsh-git-prompt#38)

@infinisil infinisil merged commit f4a8b3d into NixOS:master Mar 8, 2019
@infinisil
Copy link
Member

Oh, next time you should format your commit messages according to the Contribution Guidelines, also linked to from the PR template.

@qolii
Copy link
Contributor Author

qolii commented Mar 8, 2019

Do you mean about the second commit? (Although I see I left a period at the end of the first commit, so, noted.)

What should one do for subsequent commits? Repeating the package name and summary of the PR in every commit seems unhelpful? (But happy to do that if that's the convention, of course.) Or should you squash everything to do with one Nix component into one commit?

@qolii
Copy link
Contributor Author

qolii commented Mar 8, 2019

(Also, thanks!)

@infinisil
Copy link
Member

infinisil commented Mar 8, 2019 •
edited

It's so that in the big git log of all of nixpkgs you can easily identify things or grep for them, so these repeated prefixes aren't something bad. All the commits should be of the form <package-name>: <action>

@qolii
Copy link
Contributor Author

qolii commented Mar 8, 2019

Ahhh, that makes sense. Thanks! Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@infinisil infinisil infinisil approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@qolii @infinisil @GrahamcOfBorg