It's not your job to find bugs. If you do that's great, but the author should have done that. It's your job to ensure the code is well structured, understandable, and follows team norms.
Ask questions if you're unsure of why something was done the way it was. Sometimes even just asking "why'd you do this" will spark a great design conversation. And if something is confusing it's worth exploring more clear ways to implement it.
Always phrase things in a positive way. No matter how good or thorough your review is, phrasing issues in a positive way and pointing out good things will make discussion about the code so much easier, and also obviously make the human interaction at work nicer.
Also there may be novice people or people who just didn't do a very good job with their pull requests. Don't judge. Remember that you are not grading an exam, but reviewing is a cooperative task.
I would encourage you to try get others in on the reviewing effort.
Which Western country's democracy is being subverted by China with my data, and how? I genuinely want to know.
I know Facebook, Instagram, YouTube, Twitter, Reddit, and others target citizens and radicalise them to destabilise countries. Their private data is also sold to "think tanks" who work with politicians to influence people to vote in a specific way. How is China influencing Western democracies. Tell me. Link to articles so I can read up on it.
Well for starters, having one person be the reviewer is not a great idea.
One reason is that you have a single point of failure, as you are now discovering.
Another reason is that it creates a codebase that reinforces one persons biases.
Thirdly it can (potentially) silo knowledge, so if you only ever work on the data layer, you have no insight into any other part of the codebase.
I would encourage you to try get others in on the reviewing effort. I’d prefer assigning people to review, rather than just hoping someone steps up. (There is a saying; If it is everyone’s job, it is nobody’s job)
The actual review is multi part,
Step 1: understand what was supposed to happen. (Was this change supposed to update a dependency? Update some text? Add a button?).
Step 2: confirm the code builds. (I ain’t reviewing shit if it doesn’t even compile).
Step 3: reading the code. You’re looking out for a bunch of different stuff; access control, function size, file size, design patterns, cyclomatic complexity, weird names, weird types, overly broad implementations, complexity where simplicity should be, threading shenanigans, lots of comments (usually bad). The list is nearly endless, but trust your gut, if something feels off that’s called a Code Smell, and it warrants further digging.
It is very ok to reach out to the author and ask why choices were made, if they don’t have a firm answer, it’s time to get concerned.
Once the code looks good, I move on to tests. Check that they build and pass.
Then start looking at what they actually do.
If a function takes an input, I like to add 3 kinds of test: Expected good inputs (like valid passwords, or correctly formatted date strings) Expected bad inputs (zeros and empty strings, malformed inputs) and lastly: Fucking chaos (+-int_max, the first chapter of Moby Dick, actual face rolling on the keyboard) a function with one test is not tested at all.
Finally, at long last, actually run the code. Verify the code does what it set out to do.
PR Approved.
I've done thousands of reviews, been a senior for the last 4 years, and a tech lead for three of those. If I could distill what I've learned into a few tips:
Make sure you understand what you know what they're doing and why they're doing it.
If something feels weird, explore it, your gut is a leading indicator of bad code.
Think about how you would know if there was something wrong with the code after its deployed.
Think about how hard it would be to change the code later on.
Those are probably my big ones for general code reviews, there's more specific ones for v&v and embedded, but I feel like these apply to everything.
This is a little less about reviewing and more code standards but I'm a firm believer in functions only doing one thing. I've caught so many bugs by enforcing that during my code reviews. Determining "only one thing" is kind of by experience where you should draw that line but I find if a function is pushing 25 lines of code or more or has more than one indent it's suspicious.
It's more work to refactor and keep things clean enforcing this, but boy has it made my life better. It also makes the code ultimately easier to review as you have to keep less in your head at once for context too.
My thought process always starts with: There are bugs / bad patterns / typos / non-standard blocks / whatever here, I just need to find them.
That's better than thinking, "there's probably nothing wrong with this" / "looks good to me". If you start from the place of knowing something is wrong, you're more likely to find something, IMHO.
This sounds like a perfect moment to level set expectations with your senior, clear up anything that’s hazy, and establish a set of required steps while performing code reviews.
For shits and giggles, I had a go at writing a script that could rewrite all the commits in a branch with LLM generated commits. Couldn't work out how to do the commit changing, tried cherry picking, but it doesnt work well for conflicts (had no idea that was possible). But, I can report timings at least.
On my M1 16GB macbook air, each commit was summarised in about 10s. So its a plausible approach if you just do all your commits, and then run a script to fix up the messages, go get a glass of water and you'll come back to automatic messages.
If it's of interest to you, I can share the script, its a starting point, but I am pretty ashamed of it :D
How does their website have such a fancy hero animation but no picture of the pentagons??
No shit
There is hope, that they might run out of managers, though /s
Oh ya. We are all fucked for sure.
China isn't subverting democracy?
Chinese steal my data and be able to do nothing meaningful with it.
Is this satire?
I wonder if that'll have downstream effects like you ask it to name everyone who went to his highschool and it errors out since his name comes up.
I encourage you to look at the underemployment rate. Most likely many aren't even in their own field.
These kinds of AI output should not be protected by anything like Section 230. It is their own words and they should not get a pass for what essentially amounts to libel. Google needs to be reeled in. The fines have been too small and the damage has been too great. This isn’t even the first time something like this has happened. They have made up claims like this against plenty of other people. Claiming that they get a pass because it is AI is just using the LLM, their own LLM, as a scapegoat.
Slightly different but related https://www.eff.org/deeplinks/2022/08/googles-scans-private-photos-led-false-accusations-child-abuse
Anthropic have engineers stationed in NSA offices
Just like facebook!
. . . err
They actually do move... cities/towns/place names expand and contract, and incorporate and unincorporate all the time... it's a huge pain in the ass for our system at work. Some place names are also 100% made up by the USPS and only used by them, and guess what, you still gotta support it. Even the professional paid databases don't contain every possible place name people use.
And my comment was more about geocoding of exact addresses, not whole cities.
Same as the US. Somehow, I'd rather have the Chinese steal my data and be able to do nothing meaningful with it, than have the US steal it and subvert my democracy.
I think you should use a DAM instead. I use digiKam and it works really well. When I import photos I create a sub-folder with the YYYY/YYYY-MM - Subject, and rename the pictures to YYYY-MM-dd_hhmmss_ms.ext. Then I tag the photos with as many tags as I want and make sure they're geo-tagged. I then go through and cull the photos with the picker and delete the photos that don't make the cut. All of the metadata is written to a sidecar file, so I never change the actual image file. You might even be able to rename the photos to include a main tag.
ETA:
I recommend DK because you're already talking about touching every photo by hand anyway. DK would seem to make the most sense for the file name changes you want to make. You also mentioned Darktable. While this program has tagging/renaming (DAM) abilities, it's primary use case is as a photo editor similar to Lightroom, but, admittedly with a learning curve. DigiKam is primarily a DAM with photo editing capabilities. Ideally, one would use dK for the DAM, and DT (running in memory mode, or no DB) for editing.
The article makes general assertions about systems. It gives plumbing, software, and machine learning as examples, and I simply add the examples of human learning and human societies.
An agent specifying the criteria of a test inspired by the real world and then evaluating the performance of a bunch of other agents is equally information-incestuous when those agents are LLMs as when they are humans. If the observations of the article must hold true for LLM training, then they must hold true for humans. If they do not hold true for humans, then they do not prohibit the existence of LLM training programs that likewise avoid the decoherence trap.
Given the fact that evolution was able to find human creativity and art as forms of cohering enrichment of human performance beyond the limits of the training data of a world without art, I would expect that there is a good chance tens of thousands of engineers and billions of dollars of compute will be able to find some form of abstract expression and learning as a form of cohering enrichment of LLM performance beyond the limits of the training data of a world without that abstract LLM expression.
I thought it should be fairly obvious to people not to rely on code run from Chinese servers in general for security or privacy.
Also, not any worse than the US based services, who already work with US government directly and via secret courts and backdoors -- they also do not report vulnerabilities, and in the case of Claude, will allow you to find zero of them.
In general though, you should be running these on your own servers or compute you control or can trust.