About LDEV-1565, as you can see we already took in that pull request to 5.3 a while back, but not into 5.2 yet.
this got forgotten because the ticket still had status “backlog” instead of “in development” and “only” priority “Major” and changed to “develop” now and we will take in in into 5.2 as well.
Taking this page engine in, means at least 1 month of additional work do be done including a lot of testing until it can be go into a beta version.
As i also pointed this out in the comment of the pull request itself.
We still have the LuceeLang itself on the back burner, but it simply seems no big interest in this by the community.
Then having this in, is only the start, it also needs documentation, a website and so on.
LDEV-694 has no pull request, but i will contact Paul to see if we can figure this out.
Yes we love outside contribution, but please have in mind that a contribution never comes for free to us.
It always takes time to review the pull request and we always have to think about priority of a PR we take in.
But there is a fundamental problem with every open source project that I’ve seen (and I’ve seen many), and especially the ones with scarce resources.
Reviewing PRs take time, and huge PRs like the aforementioned #39, which makes many changes to core classes will take a long time. The bigger problem is that after some time, the base branch changes, and then the PR can not even be merged easily, which makes things even worse.
On a personal level, I can relate to the sentiments in this post, and can absolutely understand users like Jesse, who has spent many hours on that PR, I am sure.
Some of my best PRs were lost in a similar fashion. For example, I had a PR that will manage the file extension .cfs as a cfscript, without the need of wrapping the code with a <cfscript> tag. Or another one that took a Java source file and compile it on the fly and add it to the classpath. Those two were lost in some git repo back in the Railo days, and I have spent many hours on them, so believe me that I know how frustrating it can be.
The solution that worked for me, was to start by submitting smaller patches, getting feedback and preparing my future patches accordingly. Smaller patches are easier to review, and are less likely to break something major, which is the number 1 deterrent in accepting a pull request.
And another thing: there are different types of PRs:
fix a known bug
introduce new APIs
a mix of 1 and 2 above
Type 1 is easier for us to review, because we know that if it solves the problem then we can merge it.
Types 2 and 3 introduce new APIs, e.g. adding new arguments to functions, or new attributes to tags, or new tags/functions altogether. These changes needs to be discussed and approved first, before even looking at the code.
That is another issue with PR #39 which introduces a major change that was not discussed before. It is also an issue with PR #450 which adds a new attribute. Is adding that attribute the best way of solving this issue? Is that the best name for that attribute and its acceptable values? Possibly, but now I need to look into that as well, which adds another layer of complexity, which translates into developer time, which is scarce… You get the point.
API changes should be discussed and approved beforehand so that PRs can be evaluated on the merits of the implementation only.
thanks for going thru the PRs and taking the time to respond in depth, it’s really appreciated.
happy to discuss the cfzip compress option in the new thread i created. I wanted to get things moving with that as I need it myself for work and honestly, doing the leg work with that initial PR got things happening.
I clearly stated that I needed help and was out of my depth, so I don’t think a snarky review comment was at all warranted . I did all the dull boiler plate work to get things closer to the point it could be completed and if the attribute needs to be renamed, so be it, I honestly don’t see the problem, I’m happy to do that leg work of renaming some bit n pieces!
sorry about my snarky comment about the acf 2018 stuff, I’m doing this lucee stuff in my spare time too!
Yes, the manner in which pull requests are dealt with is as important as getting them merged. All responses should be courteous and all pull requests should be responded to, even if they’re not going to be worked on immediately. Something like:
Hey @jobbloggs, thanks for the PR! We’re massively busy so don’t have the time to look at this in detail, but making Patrick aware so that we can fit an appropriate priority in with the work we’re doing. Will keep you posted…
Even “bad” pull requests that merge the wrong branch, miss detailed descriptions, etc. should be dealt with helpfully:
Hey @joblogs, thanks for the pull request. Unfortunately, it looks like this is targeting the wrong branch / is lacking detail / whatever. We’re going to have to close for now, but please re-open with the [bad things fixed] and we’ll look into it…
Thanks, everyone, for this discussion. Really important. Beyond the details discussed back and forth here in the post, I wanted to note for the record that we’ve got a dedicated resource assigned to this now (@JoyMiller), and she and I are now regularly tracking and reviewing PRs, our responses, etc.
