I agree with this. The popup scrollable bug we have all been fighting to no avail appears to be the result of a one line/one word change that has created more pain than most large commits have created. So "simple" or "easy" does not mean "impossible to get wrong" or "not buggy". Going through review and most importantly thorough testing, no matter the change is better than not.
On Mon, Jun 25, 2018, 11:29 AM Mike Blumenkrantz < michael.blumenkra...@gmail.com> wrote: > I think it's a bit misleading to look at this patch (or any so-called > trivial patch) and say "this is small, let's skip review". Please consider > these points: > > * Even very small patches can create bugs. I think we have all seen this at > one point or another, but when the author views something as "easy" or > "trivial", less thought/testing goes into the work and so it becomes easier > for unintended side effects to occur. Having someone else look over the > work can help to mitigate these types of issues. > > * Patch review improves and increases community interaction. This is > indisputable: if you require interaction with other people in order to have > your patches merged then you will interact with other people if you want > your patches to be merged. In the absence of active discussion on the > mailing list (usually reserved for large scale items), patch review may > well be the only interaction that a community has amongst its members. By > not participating in patch review or deciding not to submit some patches, > you are effectively excluding yourself from the community. > > * Patch review leaves a paper trail and provides more information about a > patch and its intent. It may be the case that someone reading code long > after it has been landed has a question about it. The commit log may be a > bit ambiguous (though hopefully it is not), and so perhaps the original > submission and related review will yield more details about the reasoning > for the code. Similarly, it becomes possible to see all the patches in a > stack in patch review, which can provide additional context as opposed to > just reading git log. > > To conclude, I believe that while patch review is indeed optional and > anyone can indeed commit anything at any time after being given commit > access, taking this sort of action and circumventing the patch review > process is not only antisocial, it's anti-technical. > Everyone should choose to engage in patch submission/review because of the > benefits that the process provides, even if the result is that patches > reach the repo more slowly. > > On Mon, Jun 25, 2018 at 7:52 AM Hermet Park <hermetp...@gmail.com> wrote: > > > Actually, that will be better If patches could get in after good reviews. > > However if the patches are very small trivial changes just like this case > > (no logical changes, fix just typo, fix simple compile warning, etc.. > > well I feel it's too burdensome than we can obtain. > > > > On Mon, Jun 25, 2018 at 6:01 PM, Carsten Haitzler <ras...@rasterman.com> > > wrote: > > > > > On Mon, 25 Jun 2018 14:48:54 +0900 Hermet Park <hermetp...@gmail.com> > > > said: > > > > > > > Hmm... do we really need to get a review for this kind of small fix? > > > > At least, committers are proved, definitely know some patches are > just > > > > about trivial fix, not necessary get a review. > > > > > > if they wish to make work for themselves and others ... they are free > to > > > do so. > > > review is not a requirement in efl or e etc. if you have commit access. > > > > > > > > > > > On Mon, Jun 25, 2018 at 2:26 PM, Marcel Hollerbach < > > > > m...@marcel-hollerbach.de> wrote: > > > > > > > > > hermet pushed a commit to branch master. > > > > > > > > > > http://git.enlightenment.org/core/efl.git/commit/?id= > > > > > c44c1e2ea077dc689f52239ff341b546e95f2480 > > > > > > > > > > commit c44c1e2ea077dc689f52239ff341b546e95f2480 > > > > > Author: Marcel Hollerbach <m...@marcel-hollerbach.de> > > > > > Date: Mon Jun 25 14:25:52 2018 +0900 > > > > > > > > > > efl_gfx_path: make counters unsigned > > > > > > > > > > Summary: > > > > > we are comparing to unsigned number, and the number are moving > > > strongly > > > > > from 0 up. > > > > > Depends on D6380 > > > > > > > > > > Reviewers: devilhorns > > > > > > > > > > Subscribers: Hermet, cedric, #committers, zmike > > > > > > > > > > Tags: #efl > > > > > > > > > > Differential Revision: https://phab.enlightenment.org/D6381 > > > > > --- > > > > > src/lib/efl/interfaces/efl_gfx_path.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/src/lib/efl/interfaces/efl_gfx_path.c > > > > > b/src/lib/efl/interfaces/efl_gfx_path.c > > > > > index ca3a842b99..97439c0fee 100644 > > > > > --- a/src/lib/efl/interfaces/efl_gfx_path.c > > > > > +++ b/src/lib/efl/interfaces/efl_gfx_path.c > > > > > @@ -335,7 +335,7 @@ _efl_gfx_path_interpolate(Eo *obj, > > > Efl_Gfx_Path_Data > > > > > *pd, > > > > > { > > > > > double *to_pts = to_pd->points; > > > > > double *from_pts = from_pd->points; > > > > > - int i, j; > > > > > + unsigned int i, j; > > > > > > > > > > for (i = 0; cmds[i] != > > > EFL_GFX_PATH_COMMAND_TYPE_END; > > > > > i++) > > > > > for (j = 0; j < _efl_gfx_path_command_length( > > > cmds[i]); > > > > > j++) > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Regards, Hermet > > > > ------------------------------------------------------------ > > > ------------------ > > > > Check out the vibrant tech community on one of the world's most > > > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > > > _______________________________________________ > > > > enlightenment-devel mailing list > > > > enlightenment-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > > > > > > > > -- > > > ------------- Codito, ergo sum - "I code, therefore I am" > -------------- > > > Carsten Haitzler - ras...@rasterman.com > > > > > > > > > > > > -- > > Regards, Hermet > > > > > ------------------------------------------------------------------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > _______________________________________________ > > enlightenment-devel mailing list > > enlightenment-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
