Sunday, April 17, 2022

Comments on 'Predicting Developers’ Negative Feelings about Code Review' article

 Link to publication: https://research.google/pubs/pub49023/ 


> modern code review provides many benefits ..., including finding defects , knowledge sharing , and improving software maintenance

Absolutely agree, this 3 items are equal in importance. Even most people think that code review is all about "finding defects". 

> During code review, developers critically examine each others’ code to improve its quality, share knowledge, and ensure conformance to coding standards. 

please focus on bold word - critically. Reviewer should not praise any code he get in review. There should be detailed review with question in mind "Why we need this ?", ...

> Of the common associated factors, four that could be considered causes of pushback in code review were comments on code style, the tone in which feedback was delivered, low familiarity with the code base, and lack of rationale for suggestions or requests made

Yes, agree for all points. BUT people need understand that written communication is considered a bit more rude even writer does not mean anything bad or rude. Feedback should be considered as strive to improve for common benefit/quality (not for benefit of code author!!!). Common code base should serve for all people and NOT for certain person. If reviewer is not familiar with code, author should help to explain all/most or find another reviewer.

> ” Another respondent explained, “there are a lot of reviewers that are unreasonable and take their criticism far beyond what’s documented in the style guide.”

Better to explain in team that style guide can not cover all, even Google Style guide covers most basic items. Author of code should consider seriously feedback from reviewers as it shows exactly a way how this code will be read by other people in future. MAIN POINT: we should no have code that is good only for one person, this will force others to not participate in this project.
One more time: if you allow somebody to merge code as is because your are afraid to "push back" on author because you think he might not contribute to project in future. You do a way more damage to maintainability, because you allow one person to poison participation for others and this person will leave a project for sure because all people eventually stop contributing as their focus and interest is changing.

> “treating code review like a discussion on a web forum, i.e., ignoring the other person as an individual and arguing overly aggressively.” This theme aligns with findings from the interviews, during which participants framed aggressive comments as an initial factor often contributing to feelings of pushback

again, push back is happening to gain more quality of code or more knowledge . Do not afraid push back, it is happening for good. Everyone will benefit from push back, even pushing back is not very pleasant. 

> “it was annoying that he was not familiar with the codebase and was questioning stuff about business logic and naming.”

Reviewer is right and author need a bit more patience with reviewer. As code is review is for "knowledge sharing" . If author invest a bit more time in reviewer to explain code base or logic later on this reviewer will catch "defect" in code. 

>Several respondents expressed frustration when reviewers requested non-trivial changes without providing a justification. For example, one respondent described how, “some of the comments seemed in general hard to understand, and required a chain of replies from the reviewer/author until the reason why the change is asked is explained,” concluding, “this prevents the person asking for review from learning.”

Most of time justification is skipped when reviewer think it is very obvious. Do not allow frustration to grow in you, ask reviewer to explain. We all people, there might simply distraction happened on reviewer side and he did not finish comment. Call reviewer to get more details, verbal communication is always a way more friendly and detailed.

Some people might be frustrated, but code base is benefiting! Good code will attract more contributors again and again. Even if you lose 20% of contributors due to over high demands in Pull Requests, it is good, as 80% will stay with you.

> Situation 3: An author requests review for a change; the reviewer takes a long time giving rich, detailed feedback; the author then spends substantial time addressing all reviewer comments. All of this is done in just a few rounds of review.
However, situation 3 doesn’t create any of these negative feelings of pushback. It seems developers don’t mind spending an extended time shepherding a change through the process when they receive feedback (even if it came from a very time-consuming review) as long as it is in a succinct number of rounds of feedback. This may be due to reviewers being especially clear about what they want in their initial feedback.

This point works differently in open source and paid jobs. In open source time of reviewer is usually is more limited than time of contributor.
Review process should designed to prioritize review and not allow long waiting and long to-and-from in review. There numerous ways to avoid this (verbal calls, not big PRs, clear testing, PR-preview, ....).

>For instance, the bot could suggest that discussions could be taken offline, that another expert be brought in to provide an outside perspective, or that a change should be broken into smaller parts

yes, yes, yes.

> rejection is important because it helps maintain code quality and that success in contributing to open source depends on being able to handle rejection

yes, it is unfortunate but it is required.

> prior researchers have studied newcomers to open source projects about their initial experiences, finding that newcomers sometimes feel their initial contributions are unwelcome.

unfortunately yes, nobody wants to spend time to explain basic stuff, no that much people can coach others or explain stuff. Take it as part of unfair life and keep pushing to pass newcommers period. 

Monday, February 15, 2021

How to fix time in name of files from Google camera application from UTC to localtime

 copy all files in test folder


# to rename all jpeg

jhead -nPXL_%Y%m%d_%H%M%S *.jpg


# do only mp4 as jpeg is better to be done by jhead

exiftool -d 'PXL_%Y%m%d_%H%M%S.%%e' '-FileName<filemodifydate' *.mp4


# sources are at https://gist.github.com/romani/e2358e3c1dece784fae9fe19be0fd143

./split-img.sh


------


# sometimes required to https://gist.github.com/romani/862f33b98eb05060245ef864115ee66f 

./move-portrait.sh


in case re-split, source of wisdom:

find . -type f -mindepth 2 -exec mv -i -- {} . \; 


Friday, May 15, 2020

How to pass "diff wall" caused by reformatting or bulk change

Digging git history in terminal 
https://medium.com/@drdmason/git-blame-all-the-way-back-62957f8407

Digging git history in Github
https://help.github.com/en/github/managing-files-in-a-repository/tracking-changes-in-a-file

Digging git history in IDEA
https://www.jetbrains.com/help/idea/investigate-changes.html#annotate_previous_revision

Friday, February 7, 2020

My actions of mac setup from scratch

1)
remap "End" key if key board is long
https://apple.stackexchange.com/a/16137

2) Show path and statis bar
Open finder, in menu "View" click on "Show ...."

3) show hidden files
https://ianlunn.co.uk/articles/quickly-showhide-hidden-files-mac-os-x-mavericks/
"Show/Hide Hidden Files the Long Way"

4) Show path in title
defaults write com.apple.finder _FXShowPosixPathInTitle -bool true

Install Homebrew (https://brew.sh/)
or https://iterm2.com/
update profiles for "Default" , choose presets ".. Natural ..."
and make Terminal to keep whole history of output
Once in the Profiles section, go to the Terminal tab in the right panel of the settings, then go down to the Notificationssection and click the Silence bell option.

5)  install Sublime
sudo ln -s /Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl /usr/local/bin/subl

6) Disable swipe between pages to avoid conflict with horizontal scrolling
https://apple.stackexchange.com/a/33009/371076

7) full clock

utc clock in menu bar(if required): https://github.com/netik/UTCMenuClock/downloads



8) use bash 
chsh -s /bin/bash
disable warning on zsh usage - https://support.apple.com/en-us/HT208050  to ~/.bash_profile or ~/.profile:
export BASH_SILENCE_DEPRECATION_WARNING=1
install https://github.com/magicmonty/bash-git-prompt , update ~/.bash_profile to use GIT_PROMPT_ONLY_IN_REPO=0 as I like time in prompt.


Friday, November 8, 2019

How to list all changed files between two branches that does not have common ancestor

If tree is like this:

Same tree you can see by(if you do not have gitk): git log --all --graph

to get a all files:
$ (git diff --name-status master...staging && \
    git diff --name-status $(git merge-base  master staging) fb-4) \
  | cut -f 2 | sort | uniq
2-2.txt
2.txt
3.txt
4.txt


Good picture that explain difference between "..." and ".." - https://stackoverflow.com/a/46345364

Tuesday, November 5, 2019

How to keep release branch commits in history but do not keep branch

Command: git merge --no-ff -s "ours" release-branch 
Removal of branch label from git tree: git branch -D release-branch

It will make "diamond" in git history BUT it will not merge any changes from release branch to master. Blue dots are former "release-branch"

Tuesday, October 22, 2019