lot of good points. thanks guys.
Quote:
Originally Posted by jjshabado
I mean giving approval.
You learn by doing and reviewing code is a skill that can be developed. Being told it’s on you to review and approve code also breeds a culture of developer responsibility and trust which I think is really important.
I think you guys might also over emphasize the magnitude of difference between a junior developers review and an experienced developer. Hell, ITT we’ve had two people say they don’t even really try to understand the code and instead are looking for more superficial things.
There’s also a certain amount of cultural context here. If you’re in a fast moving company with solid test/build/release/monitoring processes and tools it’s rarely a big deal to release a bug (and if you’re working on something where it would be a big deal - obviously you require more stringent reviews and testing). It doesn’t make sense to have every line of code block on a senior person and it probably doesn’t make sense to have them do all that work too.
my company is quite large and we are absolutely terrified of bugs. like, we have tons of testing and it takes a long time to put out new releases. at least a month from checkin to production.
your points about culture and responsibility (and as a corollary developer learning) makes sense. while there isnt much difference between myself (more junior) and most of the seniors and leads, our code reviewers are certainly a step above.
but we have had issues with PR sitting for awhile so maybe it would be a good idea to groom some more devs to become competent at PRs so the project is confident in them.
I still think there needs to be a fairly high level of understanding to have approval authority unless there is a way to sort PR by complexity and importance. And perhaps such a system should be implemented.
Quote:
Originally Posted by Larry Legend
Testing the functionality of the code isn't something I do 100% of the time, but it definitely happens more than 50% of the time.
We have dedicated QA but I will regularly find obscure bugs that they dont.
A new guy on my team struggled with a piece of functionality for about 2 weeks, when he finished I tested it and found a really obscure bug that was recoverable. It is unlikely to be triggered much for the first several months or year in prod. QA passed it. I showed him the bug but told him to not worry about it for now (because we have other priorities than a recoverable obscure bug) but it is still a bug that I found instantly and QA never did.
When I read someone's code, I can immediately see things that may be totally fine, or might be a place where a bug lies, testing the end experience is a great way to determine which it is.
It also allows me to become very knowledgeable about our products, we recently released a new app that is fairly simple (30 components, 10 containers) and i have 90%+ of it in my memory and know every feature.
this makes sense. I think I may start doing this on my own to develop my own knowledge.
still dont think you should be running tests. I mean, they should all pass or its not a PR.