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

git-ftp: init at 1.4.0 #27177

Merged
merged 1 commit into from
Jul 7, 2017
Merged

git-ftp: init at 1.4.0 #27177

merged 1 commit into from
Jul 7, 2017

Conversation

tw-360vier
Copy link
Contributor

@tw-360vier tw-360vier commented Jul 6, 2017

Motivation for this change

git-ftp is a nice tool when developing for web and hosting is not under your control.

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
    • macOS
    • Linux
  • 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/)
  • Fits CONTRIBUTING.md.

src = fetchFromGitHub {
owner = "git-ftp";
repo = "git-ftp";
rev = "2c22f5118d61070449532a524db77bf810c13b97";
Copy link
Member

Choose a reason for hiding this comment

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

Please use the release tag instead of the revision.

@pSub pSub added the 8.has: package (new) This PR adds a new package label Jul 7, 2017
src = fetchFromGitHub {
owner = "git-ftp";
repo = "git-ftp";
rev = "2c22f5118d61070449532a524db77bf810c13b97";
Copy link
Member

Choose a reason for hiding this comment

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

should be:

 rev = version;

make install${lib.optionalString (!stdenv.isDarwin) "-all"} prefix=$out
'';

buildInputs = lib.optionals (!stdenv.isDarwin) [pandoc man-db];
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 add a comment, why darwin does not support pandoc and man-db here? How is man-db used in the build process?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

man-db builds only on platforms.linux and i thought since there is no man-db there is no point in building the man pages on Darwin.

Copy link
Member

Choose a reason for hiding this comment

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

afaik manpages are still usable on darwin.

};

# fixupPhase is needed for manpages
phases = ["unpackPhase" "installPhase" "fixupPhase"];
Copy link
Member

Choose a reason for hiding this comment

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

We usually prefer dontBuild = true; over overriding phases.

@Mic92 Mic92 merged commit 515d5d5 into NixOS:master Jul 7, 2017
@Mic92
Copy link
Member

Mic92 commented Jul 7, 2017

Thanks!

@tw-360vier
Copy link
Contributor Author

tw-360vier commented Jul 7, 2017

thank you!
keep up the good work!

@tw-360vier tw-360vier deleted the git-ftp branch July 7, 2017 15:14
timokau added a commit to timokau/nixpkgs that referenced this pull request Apr 26, 2018
27177 was merged but not backported to 2.7.
There is currently an open PR for 25750.
timokau added a commit to timokau/nixpkgs that referenced this pull request Apr 26, 2018
27177 was merged but not backported to 2.7.
There is currently an open PR for 25750.
timokau added a commit to timokau/nixpkgs that referenced this pull request May 28, 2018
27177 was merged but not backported to 2.7.
There is currently an open PR for 25750.
FRidh pushed a commit that referenced this pull request May 28, 2018
27177 was merged but not backported to 2.7.
There is currently an open PR for 25750.
FRidh pushed a commit that referenced this pull request Jun 2, 2018
27177 was merged but not backported to 2.7.
There is currently an open PR for 25750.
@infinisil infinisil mentioned this pull request Mar 15, 2020
1 task
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants