I'll edit the commit message before merging.

> Why even post a PR

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

> if you're reluctant to even minor suggestions?

I learnt that refactoring should focus on doing one thing well. Making multiple 
changes at once increases the risk of bugs, and makes it harder to review.

If I had written this code, then these review suggestions are fine and I would 
make the changes (I already thought of the intermediate variable and decided 
not to do it as explained above).

> why are you even doing this change at all if there are other refactoring of 
> higher priority?

I felt like removing this macro. Removing this macro is more important than 
refactoring assign_cmd.

Also, I don't always have access to my development machine ATM, so this is a 
factor for me.


Sorry you feel you wasted your time. When reviewing code though, it's good to 
bear in mind the difference between new code and refactoring. Also suggesting 
small changes is actually adding work for a volunteer, consider whether the 
changes are good enough to accept without the suggestions implemented. Working 
on dlang, I often saw 'I suggest doing X, but it's not a blocker'. This keeps a 
good balance between submitter and reviewer. Let's be pragmatic.


-- 
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-520956596

Reply via email to