saltstack-formulas / php-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
57 stars 232 forks source link

add support to install xmlrpc with xml module #175

Closed scambra closed 5 years ago

scambra commented 5 years ago

Added to map.jinja so they can be installed with php:ng:modules Should I add a state in php:ng for each extension?

aboe76 commented 5 years ago

@scambra please do if you can.

sschne commented 5 years ago

Hi,

i just added my Pull request #177 for bz2 and dba support. So this pull request here would supersede it. I checked the map.jinja and saw that the lookup is only added for Debian, so this patch would not support Ubuntu / Redhat etc. Can you also add the support there ( check #177 for comparison )?

scambra commented 5 years ago

@pather87 I think your pull request is better, supports other distributions, probably better to add xmlrpc to your pull request. I see SUSE installs different packages for xml extension ['php5-xmlreader','php5-xmlwriter','php5-xmlrpc'], so maybe it's better to add 'php' + phpng_version + '-xmlrpc' to Debian and Ubuntu, I don't know if Gentoo or Redhat has separate package for xmlrpc

myii commented 5 years ago

@scambra Thanks for this and your comments regarding #177. I've used your changes from this PR as mentioned in https://github.com/saltstack-formulas/php-formula/pull/177#issuecomment-506979472. You'll see your contribution merged in this commit: 5e04187.

Now, you may want to just close this PR. However, ideally you can continue from where we've got to with the merge of #177 and massage this PR into providing xmlrpc in a similar vein. That would be really appreciated. If you are interested in doing that, I can help extract the useful bits left over from the combination done in #177.

scambra commented 5 years ago

@myii how do you want to add xmlrpc? Add new line to pkgs for xmlrpc as I did? Or add xmlrpc as package dependency to xml line, as it's done for Suse?

Do you know how can I check where is php xmlrpc extension in Freebsd, Redhat and Gentoo? I know Arch has xmlrpc extension in php package

myii commented 5 years ago

@scambra Let's ask the main maintainers of this particular formula. @aboe76 @alxwr How would you like to proceed in terms of the previous comment?

aboe76 commented 5 years ago

@scambra most of the time, look in to the distribution package repo is there is a package for the extension, ,or check when php is installed if the module is enabled/available...

php is a tricky in the sense that distribution have been packaging this in al sort of ways..

scambra commented 5 years ago

I have redo this PR to add xmlrpc package to xml module, as it's done in SUSE.

n-rodriguez commented 5 years ago

@myii I think it's mergeable, do you?

myii commented 5 years ago

Sorry @scambra, I didn't notice the new commit. Thanks for the heads-up @n-rodriguez.

It's only failing the commitlint, so I'll fix that now when merging. @scambra This repo uses semantic-release so we need to format our commit messages a certain way. See https://github.com/saltstack-formulas/php-formula/blob/master/docs/CONTRIBUTING.rst#commit-message-formatting for more info.

So changing the commit message:

-add xmlrpc package for xml module, as it was done for SUSE
+feat(map): add xmlrpc package for xml module, as it was done for SUSE
saltstack-formulas-travis commented 5 years ago

:tada: This PR is included in version 0.38.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

myii commented 5 years ago

@n-rodriguez And now all of our warning messages from #185 need to be updated because we've got v0.38.0 now... I think we'll wait until we get closer to the php deprecation first.

n-rodriguez commented 5 years ago

@n-rodriguez And now all of our warning messages from #185 need to be updated because we've got v0.38.0 now... I think we'll wait until we get closer to the php deprecation first.

can we extract the target version (0.38.x) in pillars? next to the mute_warning and friend.

myii commented 5 years ago

@n-rodriguez Good idea, we could put something in the map. The only problem is that the README would still have to be updated manually.

n-rodriguez commented 5 years ago

The only problem is that the README would still have to be updated manually.

sadly :/