[Github-comments] [geany/geany] build.c: ASSIGNIF -> assign_cmd (#2256)

elextr notifications at xxxxx
Tue Aug 13 23:28:48 UTC 2019


@kugel- said:

> Why even post a PR if you're reluctant to even minor suggestions? 

There is nothing wrong with making suggestions for improvements "while you are there" but it also should be ok for contributors to say "not just now".  The Geany crew (me included) have a tendency to make perfection the enemy of improvement.

Of course we should object to bad changes, changes that cause problems or are poorly implemented, even changes that we simply don't like, but that shouldn't stop changes that are reasonable interim steps being made, even if they are not the final state we would prefer.

This change isn't hugely urgent, since the code being changed is working its only a safety net changing a macro to a function, removing the risk of local names from later changes being captured.  This would be important if there were regular changes in the code using it, but there have not been many changes there.  

But the change does make an improvement, so why not if its an itch @ntrel wants to scratch.

@ntrel said:

> So reviewers can check my changes don't introduce bugs.

Yes, please do NOT commit directly, although review and test can be annoying and takes time its important.  We don't have a big enough crew to have to do the extra effort to go back and find and fix accidentally introduced bugs.  

And we all know somebody else testing _always_ tries something we didn't think of :grin:


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2256#issuecomment-521046501
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20190813/601e83d0/attachment.html>


More information about the Github-comments mailing list