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

PHP: Added SSH2 and msgpack extensions. #106867

Closed
wants to merge 2 commits into from
Closed

Conversation

akoppela
Copy link

@akoppela akoppela commented Dec 14, 2020

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@etu
Copy link
Contributor

etu commented Dec 14, 2020

Hi, do you want to maintain these packages as well as the PHP team? If so you need to make a separate commit to the maintainer list, instructions are in the top of the file.

Also, you cleared out the PR template that contains things like checkboxes describing what's been done. That's never a good practice until your request really doesn't match the template, which it does in this case.

For instance the commit messages doesn't follow the CONTRIBUTING guidelines.

They should be formatted for example:
phpExtensions.ssh2: init at 1.2 and phpExtensions.msgpack: init at 2.1.2

Is it built on Linux? Is it built on Darwin? Just to know where to start testing this.

@ofborg build php73Extensions.ssh2 php73Extensions.msgpack php74Extensions.ssh2 php74Extensions.msgpack php80Extensions.ssh2 php80Extensions.msgpack

@etu
Copy link
Contributor

etu commented Dec 14, 2020

I've done some local builds.

The following builds on x86_64-linux:

  • php73Extensions.ssh2
  • php73Extensions.msgpack
  • php74Extensions.ssh2
  • php74Extensions.msgpack
  • php80Extensions.msgpack

The following doesn't build on x86_64-linux:

  • php80Extensions.ssh2

Maybe there's some newer version or patch that make it work with PHP 8.0? If not it should be marked as broken on PHP 8.0.

@etu
Copy link
Contributor

etu commented Dec 14, 2020

@ofborg build php73Extensions.ssh2 php73Extensions.msgpack php74Extensions.ssh2 php74Extensions.msgpack php80Extensions.msgpack

@akoppela
Copy link
Author

Hi. Thanks for quick response. That's my first PR here. Actually I didn't clear the template. What happened is that for some reason PR when I submitted the PR I got an error that PR already exist. And when I reload the page I saw that PR actually was created. I didn't notice that template was gone. My bad. I'll return the template.

@akoppela
Copy link
Author

I'm not a PHP developer myself and added support for the missing extensions we use at work. I'm in frontend team. So I don't see myself maintaining it unless there is a need for me. Hope that's ok.

@akoppela
Copy link
Author

I'll do more testing and comeback with new commits. @etu would you like me to rename commits and force push them with correct names?

@etu
Copy link
Contributor

etu commented Dec 14, 2020

I'll do more testing and comeback with new commits. @etu would you like me to rename commits and force push them with correct names?

That would be great! Thank you in advance :)

@stale
Copy link

stale bot commented Jul 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 26, 2021
@drupol
Copy link
Contributor

drupol commented May 22, 2022

what's the status of this ?

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label May 22, 2022
@etu
Copy link
Contributor

etu commented May 22, 2022

what's the status of this ?

Merge conflict and rebase with an update of commit messages to confirm to the https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md guidelines for how the commit messages should be formatted.

@drupol
Copy link
Contributor

drupol commented May 22, 2022

@akoppela Are you going to take care of this?

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 8, 2023
@ostrolucky ostrolucky mentioned this pull request Mar 12, 2023
12 tasks
@etu
Copy link
Contributor

etu commented Mar 13, 2023

This was superseeded by #220814

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

5 participants