search

modxcms / Collections

An Extra for MODX Revolution that provides for Resource Collections managed by CollectionContainer Resources
GNU General Public License v2.0
53 stars 37 forks source link

fix: Create a new collection #359

Closed marcuspoehls closed 1 year ago

marcuspoehls commented 1 year ago

This pull request checks if the argument is a string before splitting it on a given delimiter.

Related and fixes #349

theboxer commented 1 year ago

this is not a fix

christianseel commented 1 year ago

For reference and people finding this PR, here is an alternative fix committed by @theboxer: https://github.com/modxcms/Collections/commit/28f4a4c457fe10debae14f61ef25f03dd22abae9

sonicpunk commented 1 year ago

@theboxer I find your response here pretty disappointing. Someone put effort into trying to solve a problem that they found, and what you wrote is basically a slap in the face. One of the reasons that I put lots of effort in organising meetups and such was because the MODX community was a good place to meet like-minded developers and people who were very friendly and interested in helping one another. Seems things have changed.

theboxer commented 1 year ago

@sonicpunk since when is simple fact stating taken as an insult? I never use too many words. If there are questions on why I think it's not a proper fix, the easiest thing is to ask.

sonicpunk commented 1 year ago

It should have been explained why it should not be considered a fix. Also since this is a first time contributor, the person should have been thanked for taking part. Basic communication skills.

theboxer commented 1 year ago

i clearly lack those, sorry about that

theboxer commented 1 year ago

@marcuspoehls @sonicpunk

Hey Marcus, based on the input from Ben, I'm really sorry how I worded my initial response and seeing it again from a distance, it does look rude and disrespectful, even though it was not an intention.

Seems like sometimes less words are actually worse than more words, I'll promise I'll try to learn a basic communication skills, so this hopefully won't happen again.

To elaborate more on why this is not a fix, it doesn't resolve the root cause of the issue, but only adjusts the part that generates the error. The method is accepting array only as a first argument and the signature will be adjusted later in the future to have the type in it (once we decide we want to drop support for dead php versions...).

Thank you for your contribution and please do not let yourself discouraged from helping others by assholes like I was in the previous response.

-- @sonicpunk Thank you for making me realise how my approach looked for you and others.

marcuspoehls commented 1 year ago

@theboxer Hey John, thank you for reaching out again. I appreciate your message and will contribute if I find something that can be improved/added/adjusted in this package :) Cheers!

sonicpunk commented 1 year ago

@theboxer Thanks for resolving that with Marcus. I was also trying not be harsh, but just stating how it got understood. I am glad to here that you didn't intend to be condescending to Marcus.