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

php7 fixes - fix constructors and 2 common notices #15

Closed
wants to merge 3 commits into from

Conversation

SjonHortensius
Copy link
Contributor

I've fixed all old-style constructors and 2 notices I saw frequently (in classes/Misc.php). This change is backwards compatible with PHP_VERSION > PHP4 (which I hope no-one still runs)

@Skatox
Copy link
Contributor

Skatox commented Apr 14, 2016

My installation also complains due to this, this is an important commit

@ybenhssaien
Copy link

This is an important pull request !
@SjonHortensius can you please update your pull request ?

@elynnaie
Copy link

@SjonHortensius and all, I merged this PR into https://github.com/myfarms/phppgadmin. Other PRs welcome.

@SjonHortensius
Copy link
Contributor Author

@xzilla would you clarify why you ignored this and other pull requests and choose to do the same work yourself 3 years later (in 1ea83ac)?

I'd be happy to contribute other fixes - but it'd be nice if they'd actually be used :)

@xzilla
Copy link
Owner

xzilla commented Jul 16, 2019

@SjonHortensius I'm not sure there is a succinct way to summarize the state of development the last few years, but most of it was related to timing, a plethora of PRs all re-implementing the same thing (yours was not the first, how do you pick just one?), and many of the PR's conflating multiple issues into a single PR (class updates vs fixing warnings vs pgsql supported versions). Right now I'm slowly working through some of the backlog of PRs to clean up the repo, and have some changes to the DEVELOPERS file to help improve workflow (more clearly outlining feature branches). I do hope you'll consider contributing in the future. If you want to discuss more, you can find me on the postgres slack team.

@SjonHortensius
Copy link
Contributor Author

@xzilla that's actually very sensible - especially the part about not implementing multiple fixes in a single PR. You could consider looking at CONTRIBUTING to emphasize this. Also - if you'd have replied with that suggestion to any of the PRs I'm pretty sure people would have accommodated (I know I would have).

Thanks for the explanation - I'll be sure to send more PRs your way. Please consider closing other duped PRs (eg #16, #21) as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants