-
Notifications
You must be signed in to change notification settings - Fork 121
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
Feature Request: Add ACL and Database Selection for Redis #343
Comments
So, GridPane provides a PR, you take that PR, repackage it, and then merge it without any sort of acknowledgement. You also forgot to add the PR number in the changelog. You've accepted PR's in the past and acknowledged others. What happened to "Good work, Good people" @rahul286 |
@jordantrizz forget about it, i reached out directly to rtcamp and drew blanks, can't be bothered with this... we are forking the plugin entirely, they didn't even include the filters and torden fixes... We have a bunch of other improvements, but they will not be coming here.... @joelabreo227 is even the guy who apologised 2 weeks ago, and radio silence since then... |
Hi @jordantrizz -- I am responsible for coordinating rtCamp engineering efforts toward Nginx Helper. We are actively working to address existing issues on both the WordPress.org support forum and the GitHub repository. We recognize we are behind where we should be on this. As part of these efforts, I assigned @Vedant-Gandhi a few tickets in Sep 2024, including #350 #343. He raised #371 in response, which adds support for Redis Username, Password, Database and Unix Socket Path. The only similarities we see are:
Our PR addresses a way smaller scope than the parent PR, and it functionally differs from @gridpane's PR as can be seen in the diff. We also raised #377 to fix a bug we discovered in the implementation.
Given the technical context above, I'm not sure if this is a fair assessment. Happy to continue the conversation if there's something I'm not seeing. |
@joelabreo227 that is complete BS... Anyone checking the PRs can see that the code is effectively the same, including the use of constants to set and manage these. He even copied my indentation issues... seriously... Stop, you are embarrassing yourself. |
It looks as though you already apologized to gridpane, so I don't really see a need to continue, If you want to pick and choose who you label as contributors, then that's fine, just be transparent about it. This just doesn't make sense, especially since someone did a bunch of work, and you took pieces of it and the general idea without giving credit. Just doesn't seem to align with rtcamp values. |
@gridpane The core logic for addressing issue #343 / #350 has limited possible approaches. The code that was merged has parallels to your proposed code, leading to the understandable perception of copying. However, our PR was developed from scratch. I've already addressed the similarities in constant names earlier. There are also a few differences, which can all be verified in the PRs.
Ideally, we should have worked with you to update your PR to get it more in line with what was ultimately merged. However, the way it panned out, we ended up raising an entirely new PR and going from there. We will be more careful going forward.
@jordantrizz To clarify, I apologized for the negative experience gridpane is having, and promised to look into their reported claim re: incorrect/missing code attribution. You have a point though; we failed to acknowledge @gridpane's contributions via feature request + effort they put into a PR (though it was not ultimately merged). We have remedied this via PR #393 In the long term, we will also include a CONTRIBUTING.md file to make things more transparent. |
@joelabreo227 you are talking out of your ass... You took the PR, rearranged it a little and didn't acknowledge anything... I was the one who added the use of constants, all the settings are constants because that is how we use it... and the disabling the field if the constant is set... it is all my work We have been using all these same constants since June of last year in production across several hundred thousand sites... Stop talking shit man, just stop... its embarrassing, seriously We are fully forking this plugin, we will improve it with all the other ignored contributions, and remove all mention of rtCamp other than original contributors. |
Thank you for acknowledging this Joel, I'm glad that there will be more acknowledgement of contributors to this plugin going forward. |
@gridpane Constants are a pattern in the plugin for several years now. I have nothing further to this part of the conversation. In recognition of your original request and the effort that you've put into the PR, we've added you as a contributor (see #393) You are free to fork the plugin with attribution to the previous contributors, including those from rtCamp. |
@joelabreo227 Constants were for a limited number of settings. I expanded the constants to all the major inputs, which were not all included before. I also included all the new redis inputs I created, and also made the inputs inactive if the constants were used and included specific messaging as to why. All of this was used, almost verbatim, this is not a coincidence and indeed you using this above as a differentiator just shows you are not paying attention - effectively not doing your job as the person responsible for leading rtCamps efforts... We have this plugin active on over 200 thousand sites, all of these installs continued to work after your update, because it was all the same. The only thing that stopped working was the cache purge all for fastcgi because your master is still broken, and the filters for cache purge exclusions. Which are the aspects you didn't include... You were dialing it in... Of course you have nothing more to add, because you know you are wrong, you approached this incorrectly and should be embarrassed about taking people's work and not acknowledging it until it becomes a public issue and you are forced to do. I won't thank you for the contribution acknowledgement, since that is entirely expected, what you are missing is an apology. We have forked the plugin, and will of course attribute the earlier work to rtCamp and others, but in future when we pull in your updates to our version we hope you are properly attributing work because if we find out your devs are continuing to attribute others work to themselves this will become a problem. |
We're not going to be able to make any constructive progress without codebase references to back your statements. From what I understand, you are referring to constants like |
The mistake you are making is thinking that at this point I care about being constructive with you. Anyone reading here can check themselves and see. Your junior dev took the implementation and code, passed it off, made a few minor alterations, and attributed it to himself. Its dirty behaviour, we want nothing to do with you, we will take responsibility for this going forward. You can not be trusted. |
Completed work in my fork here:
https://github.com/gridpane/nginx-helper
Can set the connection parameters via wp-admin or via constants
Will create PR after final testing on our platform
The text was updated successfully, but these errors were encountered: