Differential D13123
New Port: ftp/php72-fastdfs Authored by joneum on Nov 16 2017, 7:53 PM. Tags None Referenced Files
Subscribers
Details New Port: ftp/php72-fastdfs PHP module for accessing a FastDFS cluster WWW: https://github.com/happyfish100/fastdfs PR: 223714
Diff Detail
Event Timeline
Comment Actions if it is a copy of an existing port, make sure you dont simply svn add, but svn cp :)
Comment Actions it should be $PREFIX, not $LOCALBASE, as it is used to install the port, I think :) Well, maybe no one looked at it? :) -- no one complaining does not mean you should not fix it once you see it :) Comment Actions I think you can use CONFIGURE_ENV= ROOT=${LOCALBASE}instead of the patch. As it is used in the following way PHP_ADD_INCLUDE($ROOT/include/fastcommon) PHP_ADD_INCLUDE($ROOT/include/fastdfs) setting it to $LOCALBASE seems like the correct move Comment Actions Also like with the other ports i highly doubt that this is a useful addition!
Comment Actions After update php*/fastdfs, here is an copy for the new php72 port. svn cp php71-fastdfs/ php72-fastdfs/ #PHP7.2 Comment Actions Theoretically this is good to go. However, if you'll merge all the fastdfs ports into one soon, I'm not sure if there is much point in adding it :) Comment Actions If "svn cp php71-fastdfs/ php72-fastdfs/" was correct, and there is nothing to complain about the code, I ask for release. thx :-) Comment Actions Technically the code is fine. Its just a bad solution. The correct one would be a version-free port which uses the PHP defined as default either in the ports-tree or in /etc/make.conf. Its not about getting a technical good solutions, its about getting a good solution. The code is just the implementation. It doesn't make much sense to add this port wrongly, because it just works while planning to do it correctly in the future. Comment Actions I summarize:
I'm closing ihere, telling the maintainer you're not releasing that after 2 weeks, and closing the PR too. Comment Actions Yes. It wasn't okay to have this ports in the first part. And i am consistent with myself, as you can see in some other PRs: Also please note: "after all changes" its kind of bike shredding. I answered exactly one day after tcberners comments. That is not the same like "after 2 weeks, after all changes" - its exact one day, after various very helpful tips received by your main mentor the day before. The problem here is that other committers doesn't had the insight of this problem. Which is fine - it happens to me in other parts multiple times and is still happening. But when a better solution is found, we should switch them. Here the /etc/make.conf change would be the best advise to the users requesting the port, because it works in quarterly and head and did for many years. Comment Actions Not correct. You did not say exactly what you would like to have! You have done that only today, after 2 weeks! Comment Actions I did. I wrote at 2017-11-17:
There was no answer to my question. You just came up with another svn cp at 2017-11-25 and asked again at 2017-11-28 if its okay. While clearly never picking up my questions. Which is what i stated at 2017-11-29. Even if you did miss the importance (to me) of my questions its not a good way you dealt with it. You focus on a lack of response for 2 weeks. Between my answers were 12 days and you wait 8 of them. And that all besides from persisting your frustration in a review forever, instead of just discuss it technically here or personal via email. Its okay for me, but i doubt its okay for you. ;) Also i provided the correct way to enable the user to get what he wanted. You should have added this to the PR or add me to CC to enable me to do this. Comment Actions They complain about missing answers, but do not answer themselves to open questions. What exactly is in the make.conf? Comment Actions Please read what i wrote: you complained about missing answers from me without saying something about my raised issues. So you complain about yourself, not about me. ;) Also as you know i'm not only a mentor but a teacher in real life. I don't provide full insight to give *you* the possibility to dig into it and learn from it. This helps much more than an long explanation (which besides is given in the link PRs). Happy you finally ask for at least make.conf. That is for example in my make.conf to get this port: Same for every pecl port i need. It compiles the port against the defined PHP version. The php*-fastdfs ports don't work this way, because the use PHP_VER. Stripping the PHP_VER and use make.conf reduce it to the need of one port only. Comment Actions Please do not just look for the error with me, but ask yourself if you have acted correctly. The answer should be "no". Comment Actions I never stated that i did everything right. But i try to teach you were you did wrong. As a mentor its my job to learn for myself, but to teach you. I provided points, solutions and possibility for discussion you did not use. I provided feedback for your communication in the PR you did not use. I pointed to the possibility to talk beside this Review in private you refuse to use and just don't want to discuss further. Comment Actions
I will gladly accept this advice, but you too should take your own advice. Work on your attitude. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||