Code Review Argument: “Why are you reviewing!?”

There was a code review submitted in a project branch. I had no reason to review it because I wasn’t assigned to the project, but I often like to be nosey and see what is going into the releases.

I noticed a few classic mistakes like server-only code placed in “common“ that gets installed to both client and server when deployed. There was also a caching problem where the first call will store the data in the cache, then a call from a different user will then go into the cache and grab the other user’s data! It’s the classic mistake of forgetting about how the app servers are shared between many users and many organisations.

For years I have flagged these problems up. Maybe we should have it as part of an induction process to go through common misunderstandings. Anyway, I send them on to some developers (Dean and Mark) that I have discussed issues with or joked about them in the past. Each developer then decided to also leave comments on the review even though they also weren’t assigned to it.

In a private chat, Dean said “why isn’t the lead developer spotting these mistakes and teaching them?“.

I assumed he might have actually then put an official complaint in, because Gary, the Senior Developer assigned to the project; then left an angry comment.

“why are you commenting on a project level change? I thought you were struggling for time and this is a project branch that I haven’t yet reviewed”

The thing is, he hadn’t said that to Dean and I, but to the other developer Mark. It’s like maybe they had some kind of previous arguments and now it’s flared up.

Mark rightly responded:

“You’d added comments. This isn’t the place to debate such things, please contact me by message if you have a problem”

I think it could be the case that Gary hadn’t fully reviewed it, just glanced at it, left some trivial comments, and meant to come back. In my opinion, if someone does the review for you, then doesn’t that save you a job? Many developers seem to hate code reviews since they would rather be writing code, not reading it. So really he should be grateful. But he has taken it like a personal attack like we didn’t trust Gary to point out these major flaws in the code.

It’s also beneficial to spot problems as early as possible. There’s nothing worse than aiming to get the project in a release, then when you want to merge it in, then the experts then look at the code and tell you that it has major flaws and cannot be released. At least we have flagged it early in the project and they have plenty of time to address it.

Secrets in code

Recently, we received the following message sent to the entire Software Development department from a manager:

Over the last couple of days, we have witnessed multiple instances of sensitive credentials committed into GitHub. This has been noticed by chance whilst supporting teams / reviewing PRs. Instances have included an individual’s GitHub PAT & test user account password.

The security team are likely to enable a secret scanning tool in GitHub to help identify instances of this in future. However, this isn’t guaranteed to spot all issues and by the time it does; we are already potentially at risk.

Please ensure that you are vigilant in reviewing your own / other’s code before committing / merging code. If a sensitive credential is identified; please ensure that this is removed and revoked immediately to prevent misuse of the secret.

I’m really surprised if multiple people have done this, because we keep talking about improving security, and improving processes. Adding private keys/credentials etc to your repository sounds like a common cliché and is like Security Lesson 1. From the very start of the project, we have talked about having security scanners on our code from the very start.

I thought GitHub automatically scanned for credentials but it might just be for public repositories, and ours are private.

Security keys/tokens can often easily be regenerated so it’s easily fixed. However, your mistake of committing them is in the history of the file. I suppose there’s technically ways to modify the history but at a massive inconvenience.

Remember when people used to know what they were doing?

Remember when people used to know what they were doing? those were the days.

“what concerns me the most is that there was a time where everything almost worked like clockwork and now it seems like more ruins every day”
Software Architect

“I am more surprised when something works”
Me

We used to be a company full of smart people, working effectively. Now we work slowly and people just cut corners and do incredibly dumb things. In more recent times, people now don’t think for themselves because they ask AI what code to write. Sometimes it’s absolute rubbish but they never reviewed it themselves; so it really is zero thought. You point it out to them that it’s not going to work, and they respond back with an overly polite message, clearly written by ChatGPT; which just adds insult to injury.

So it’s like developers don’t even develop because AI does it. Then they don’t do any dev-testing. Then the Testers don’t know what they are doing either.

Recently Testers have been installing our software on the application servers.

Even though one of the Lead Testers has been posting angry rants about it; it keeps on happening. The Lead Tester’s points were that it’s not representative of live, and how it takes up the RAM/processing time and lags out the app server for everyone else.

I don’t get why people got the idea to install the client on the app server, and remote on. You can’t think that is official. The servers were always configured to only allow 2 people on at once, so it’s not like the entire department can log on to test if it was the official process.

I just hate what this company has become. I feel like it’s just gonna keep getting worse with managers constantly encouraging people to use AI.

Let’s read the words, the words, the words, of the developer

Introduction

When working with Indian developers, their English skills can vary. You also need to be aware of certain words exclusive to Indian English; some of which I actually like. For example they have the word “prepone” which is the opposite of “postpone”, but in UK English, we don’t seem to have a single word for that.

Some phrases seem more like poor grammar. An example of that is “Can able” or “Can’t able” when we would say “I’m able/unable”.

“i think you can able to see the second image is it?”
“I can’t able to find any relationship between those two codes”
“still we can’t able to recreate the issue”

“For the same” is an interesting phrase because it just refers to something earlier in the sentence without having much meaning. It’s similar to when they say “do the needful” which just means “do whatever is required” but often doesn’t really add anything to the instruction; if they have requested something from you, then surely you will do it if you can.

There’s a few strange greetings like saying “good noon” which I’d assume is just a shortened version of “good afternoon” rather than being appropriate for a very specific time period. There’s a few people that have a strange greeting of “Ho!”

“Ho!! Is it please can you share those knowledge with me…”

To take time off, they like to “avail”. As a bonus, here’s a strange requests:

Morning Team,
I have picked up fever and heavy cold. Availing AL today.
 Please conduct stand up and end call.
Available over mobile for any urgent issues.
Thanks and Regards,
Jeeva

I’m glad you told me to end the call Jeeva, because I’d have stayed on it all day otherwise.

Indian Pull Requests

When it comes to the Code Review process aka Pull Requests (PRs), it can be hard to ask them why they are making certain changes. Sometimes asking questions can just lead to further confusion. Also, sometimes I’m sure some developers try to blag and hope you move on.

I was discussing this with a Lead Developer and he agreed that asking questions can either result in

Blagging
Revert the code and hope it works
Or you actually get a good answer. But then if it’s not clear why the code was written like that, then maybe it does need a code comment or some documentation so others don’t get confused in future.

Even though I often got frustrated with their comments, in recent times, a lot of them use AI like ChatGPT to rewrite their responses, or sometimes I get the impression they just put your question into the AI and hope it comes out with a good response. So instead of poorly written English, it’s all robotic and a blag of jargon. So you can’t win really.

Row

“Refresh on special while saving special note, row background, Radio button alignment based on include exclude”

Blagging with Words on PRs

I questioned their pointless try/catch blocks which were catching an exception then rethrowing the exact same type of exception.

“Yes, as I couldnt use the dll in the resourcepicker project, so we can thrown the exception and catched it in resourcepicker class”

And

“The resources can be used due to filecahe, but no changes can be saved, when service is down. The above message is already used in Picker solution.”

Then when their project was being merged into the main branch, another developer questioned the same code. This time they said:

“To restrict that, have drilled up the ux tree and displayed the error message.”

Observation

“Found an observation while testing 12602 in 9.3.6 branch”

what does that even mean? I assume “observation” means “bug” or “potential problem”.

Bad Refactoring

He refactors some existing codec but also changes the return type of the method, which means the caller’s logic will have to be changed so was causing cascading changes which weren’t really relevant to his main change. Also, the logic didn’t look equivalent so I wouldn’t call it refactoring:, more like introducing a bug. He then claims he hasn’t changed it…

Me: "is this equivalent? It was checking >1 not >=1"
Them: "Actually, I haven't attempted to modify that as the logic written working as per acceptance criteria, and it already tested"
Me: "I don't understand, this method has been changed in this PR"
Them: "Just used expression for methods as commented by Andy. Apart from that i haven't changed any logic around that."

Down Merge

Vignesh
Here after no comments fixed against assurance branch?
Just need information about down merge
 
Andy:
sorry I'm not sure what you mean?
 
Vignesh
Two comments pending for our side... if any one raise PR I will raise PR also. Because of down merge... Incase only I will raise PR again do down merge that's why I am asking

IsMobileEnabled

IsMobileEnabled needs to return boolean value, so removed exception caused by null and also the GetResources during Trigger prompting needs to include Template also along with Protocols.

Didn’t Launch The Portal

me: “where is this used?”

developer: “This is used at TryLaunchPortal()…. At this point of time we never know the portal type to compare and verify the condition because the user didn’t launch any portal“

walkie talkie comms going on here

This reminds me of walkie-talkies, stating “over” so you know it’s the end of the message.

Roshni 
give line break after method over
 
Shoban
Ok Roshni, Updated the changes
 
Shoban
Completed with the Changes
 
Roshni 
give line break after method over not before the method over
 
Shoban
Thanks Roshni, Got your point. Made Changes
 
Roshni 
and again please remove the empty line no 267
 
Shoban
Code changes completed as mentioned

Welsh

PR: Updated the Walsh text

Description: Updated the resource file with Walesh text

Do you think the text is gonna be accurate if he can’t get the title correct in English? It should say “Welsh text” as in “the Welsh language”.

Customer

Merge from Curomer first branch to main

Accelerator Keys

To define an accelerator key (allows you to use Alt key to select it), you place an & character before the letter. So Export has E defined. Edit can’t use E because Export has taken it, so they have chosen D. Cancel seemed an odd choice of N.

btnBackup.Text = "&Export";
btnContinue.Text = "E&dit";
btnCancel.Text = "Ca&ncel";
btnBackup.DialogResult = DialogResult.None;

Me
can't C be used as an accelerator key?
 
Kalyanaraman
C for Continue
 
Me
what is the continue button? Isn't this it? btnContinue.Text = "E&dit"; that is using D

SQL is up to 10 times better

yes i have tried with mocked 10 lacks data in local
and while this query the data was well optimized.
For data, I ran sp thrice

I bet you can’t tell if this is from some old children’s folk tale or an Indian’s PR

Always Run SQL Code Analysis

Roshni has worked here several years, and when she started, I’m sure she made the same mistake several times. When making a database patch, we have a Patching Tool that not only applies the patch but runs some code analysis to make sure it conforms to coding standards.

Many times when developers have reviewed her code, Roshni has been told her patch would have been flagged by the tool if she had run it as part of her Developer Testing.

When I was a junior, once I was told by the Seniors; I never forgot to run it again. It’s like the embarrassment/shame makes you remember. Also I cared about quality and this was a simple process that ensured quality and standardisation of our SQL code.

Recently, she had merged her fix ready for release and a Tester, Mick pointed out there were patching errors so her SQL patch cannot have been run through the Patching Tool, or even tested.

She claimed it had been tested, and it was a problem between SQL versions. So her claim was that – both her local machine and the test server it was run on (by another tester in her team) was a different version to what was on the main test environment we use before releasing the software.

So Mick looked at the SQL patch and saw the error was about a missing namespace. The patch was inserting XML, and XML has a namespace attribute on the first line. So then he looked at what data is currently in the table, and saw that all the existing entries had a namespace declared, and this was missing from Roshni’s patch.

So Mick embarrassingly pointed this out. So she had lied about testing the patch locally, she must have lied about it being tested in her team, and lied that it was an SQL version issue.

She then submits a brand new patch which conditionally checks if the previous patch had created the entries. If it hasn’t then, this new patch would insert them, then if it had already added them, then her new patch would run an update statement instead.

Mick then points out that this is nonsense because the original patch had failed so would have just rolled back and stopped patching. What she needed to do was just to fix the original patch so it would run. So then she quickly deletes her new patch, and updates the original one.

Although it’s what we wanted, the speed that she did it makes me think she hadn’t run the Patching Tool because it can be very slow to run. So yet again, we have told her it is important to run it through the Patching Tool, and she hasn’t bothered.

Although I think nothing was actually wrong with her new change, another tester had pointed out that her changes were across two repositories and her changes in the other repository were also flagging errors in the Patching Tool. So it’s not like she just forgot to run it once, it’s just that no matter how many times in the past we have told her YOU MUST RUN PATCHING TOOL; she never does.

It’s just infuriating we keep employing people like that that don’t listen or care about the work they are doing.

Innovation shambles

Recently, managers decided that every few months we should have an Innovation Week. The idea is that you can work on ideas that can improve our work processes or even add a new feature to our products. However, the time limit of one week is a bit limited to actually get something complete in my opinion.

To be efficient, we really need to come up with a great list of ideas before the innovation starts, otherwise it cuts into the week. Some people did submit ideas before, and others on the day.

The initial meeting quickly became a bit of a shambles. Paul had created a Miro board under a different account that the attendees didn’t have write permissions for. Even when we clicked the link to request access, and Paul claimed he approved it; it still didn’t work.

He then tried creating a different board, but that didn’t work. To not waste further time, we just posted ideas into the Microsoft Teams chat which then he transferred onto Miro.

Since the ideas were essentially just titles on the board, people were supposed to explain their ideas but I don’t think many explained too well. We probably needed some kind of formal process to:

describe the problem,
ideas on how to solve,
pros and cons,
any possible costs like software licences,
prerequisites to be able to investigate or implement the idea.

Another thing was missed is that you have to have accounts to use many of the AI tools, and that was a focus of this month’s innovation. With a lot of software, it often needs a special licence for commercial use and we weren’t advised how to acquire licences. We had Github Copilot and Office Copilot but what about other AI tools?

One guy apologised for misunderstanding that the ideas should be process improvements and he had come up with an idea for our software that our users would use. Paul said he hadn’t misunderstood at all and we could suggest either process improvements or new features… but that’s not what the Miro board said. It was only for process improvements and so all but one idea was for process.

We needed to assign our names to them, so initially Paul tried to create a spreadsheet but couldn’t work out how to share it so we could all edit at the same time. He ended up pasting the ideas into a Microsoft Teams “Whiteboard” which I had never used before but it looked like the Miro boards.

There were loads of ideas, but many were of debatable value. However, like I stated, we never discussed them effectively. Without knowing the pros and cons or prioritised the business value; there were loads of ideas that definitely weren’t strong enough. So with a large list, it was hard to pick something to work on. Some of them would need more than one person, but what guarantee is there that the team will be full? Less likely when the list is so big.

So I asked the question if we should only put our name against 1 item, or vote for several so we can see which teams are full, then the full teams get approved. Paul said to only vote once otherwise it will look like teams are full, but you’d end up dropping out if another one of your votes were successful. I suppose that’s a good point, but only voting once will mean you could be the only person to vote on a team project, so would then have to choose something else anyway, or gamble and go by yourself.

With most people finally assigned (and many just disappearing, presumably to slack off), with many going solo, and some probably having more team members than required; we got told to communicate with our team members.

I was in a team of 3 but I thought the ideal team would just be a pair. I waited for 30 mins or so but the guy that came up with the idea hadn’t contacted me, and you would assume he would take the team leader position.

I then took initiative and added a group chat with my 2 team members, and after another 1.5 hours, I finally got a response from one person who asked how we should begin to plan. I responded with my notes I had created to set the scene. He suggested one extra point to my notes, then didn’t hear from him for the rest of the day. The other team member didn’t respond at all.

The next day, my manager contacted me and said I was assigned to help finish a project that was behind schedule so my “innovating” had come to an end.

Absolute shambles really.

The Chop

I was once contacted by my manager to review a particular code change, but with the instructions to actually select the Reject option if there was anything wrong with it.

Things seemed a little odd, so I was suspicious about the request. Usually you would only use the Reject option if it was completely the wrong approach, not just if one thing could be improved.

The change was an SQL data fix, but seemed for a bug that would rarely occur, so my instinct is that it should be run manually on the afflicted sites rather than sent out as part of the normal patching process.

The normal process would mean the script would be run on all servers, and be subject to the usual slow “roll out” process; therefore delay its application to the affected site.

Looking at the comment from Support, it sounded like it was possibly just on one site.

There were 3 cases linked to it; 2 from the same site, 1 with a title of a completely different error number. Then the workaround is stated as “Re-add the Default Location to the Template” so they probably fixed it already. So maybe we didn’t even need to do anything.

Looking at the dates that it was logged, it seemed like it was classed as a minor bug so had a long time period to fix as per the Service Level agreement so I was sceptical it was still an issue after 2 years.

So my initial instinct says it should be applied as a manual patch, then reading the details from Support, it sounds like it was just on one site and they had already manually fixed it, presumably via the UI.

So I asked my manager if the site has been contacted to see if it is still a problem? Then we can just close it.

And that’s when my manager said

“His skills are currently being assessed so he’s been left to figure it out and told to ask for help if/when he needs it.”
Manager

So it seems like they have given him a bug to investigate then fix/close. Since he has struggled to resolve items before and been reluctant to ask for help, they have chosen this one to test if he is collaborating with the correct people. He didn’t, so they sacked him.

I haven’t encountered many sackings; everyone seems to imply it’s a lot of hassle, but certain managers are more willing to do it. Another approach is to declare that a “new opportunity” has come up and they will be placed in a new team to see if that causes an improvement.

Employee Profiles: Neil

Neil was very similar to Gerald. Although I always got on with Neil, his programming skills were a bit lacking.

He was definitely one of those software developers that may have been good in his prime, but the languages he has used are now obsolete and he struggles to learn new things, so was a poor C# developer. Or maybe he has always been poor.

Neil is like “I don’t know why this bug is happening, I might try change random parameters“

So I said “have you found the barcode code since it is that which is displaying incorrectly?“

“No I haven’t. I suppose that’s important to find“

Neil in the Standup update on Thursday: “gonna switch branches”

Friday: “just in the process of switching branches”

Switching branches takes a few minutes (Git, plus some config changes)

“I don’t have a good feeling about downloading nuget packages any more. Before the patch, the fix made 21 changes to .csproj file and .config files in order to work. So far it has made 523 changes.”
Neil

What’s he on about? sounds like Neil is deleting packages then removing the part of the build script that grabs them, then wondering why he can’t log in.

One trait they had is that he seemed focussed on his own work and didn’t pay attention to what anyone else was doing. So there could be well-known employees at the company and he wouldn’t know who they are. So there were plenty of conversations like “go and ask George for assistance” and he would be like “who’s that?” or “what does he do?” much to the derision of team-mates.

"might be worth getting you and Nick on a call together because you are working on similar items"
Neil: "Nick who?"
"Nick on our team"

He once turned up to the stand up which started at 9:30 but he was supposed to be on a 4 hour training course from 9. He said he joined the meeting but no one was there. Our manager was like “did you join using the correct link? The calendar invite says PLACEHOLDER so that’s not the correct one“. Why didn’t he ask people if the meeting was on? He should have messaged his manager straight away.

A few months later, he missed another meeting. “did you not see my reminder about this morning’s call in Slack yesterday afternoon?“

“I thought it was some advice on where to look in the code“

The team lead had posted “Announcement : reminder that we’ve got a call at 9am tomorrow about MEDS ISN”.

Probably Neil’s brain – “mmmmmmm MEDS ISN call. Sorry, I can’t seem to find the MedsISNCall code. Is that a 3rd party dll?”

I’m still baffled by Neil. If he thought it really was code, then why wouldn’t he ask you how to find it?

He often struggled and didn’t ask for help, even though we repeatedly told him we would help train him up in our software and with C#. He would raise it as a point for himself in the Agile Retrospectives, and say that he will ask for more help and work as a team; but then carried on as normal.

“Yesterday, I failed to log in after 3 attempts. So I am carrying on looking at it“

Neil wants to add a new user to the database because he doesn’t know how to unlock his password by running a script on the database, but it seems he didn’t know how to add users either. I wonder what time he locked it. Did he lock it at 11am, then walked off?

Neil last week: I need to ask for help more

Probably Neil’s brain this week: well, my account is locked, looks like today is a write-off

Later on, the manager asks him if he is all sorted and he said: “I can log in but an error pops up.“

So he is blocked again and never said anything. He is really trying to get himself sacked.

I’ve noticed these types of people often blag their standup updates by saying “Sent a message to…”. Sending an instant message or email to someone could take some time to make sure it is worded correctly, but it’s not really a significant thing. Then there can be times where the recipient doesn’t respond because they are busy or out of office. But then what are you doing whilst blocked? They seem to use it as an excuse to write-off the entire day and shift the blame to someone else.

There were several times where he was working on items that had been picked up by others, had completely misinterpreted the requirements, or just had general bad luck with work being deprioritised as he was working on it. It kinda became a running joke like he was a cartoon character.

We told Neil to take a bug from the backlog. It was one I had investigated and put loads of notes in so it should be really easy for him to fix. He took the one already assigned to me with a status of “In Progress”.

Build Problem

On a standup, he was saying he was struggling with a build error. I volunteered to help him since we didn’t want him stuck all day. I asked him to clarify the situation; “is it on a computer that has always worked then suddenly stopped working?” And is he “currently running the build script without any of his changes in, or could it be caused by something he changed?”. “If he types that az login command, what does it say?”

Then all he says back is

“Curtis from IT is supposed to be setting up elevated access on A20205 but it isn’t working yet“

WHO IS CURTIS? Why is he included in the story now? and what even is that machine name? Am I supposed to recognise it?

“Existing one that has always worked except for these latest build changes. Curtis said “I’ll need to switch your admin access over from Primary to an elevated to match that of the other users on the jumpbox” and under the circumstances I thought it sounded like a good idea.”

I asked why Curtis is involved. Has Neil asked him to fix his build issues? and why does he think it’s an IT problem rather than a Development one?

“He contacted me. He did not give a reason for making the change so I don’t think he knew about my build issues. He may have noticed that others users have elevated permissions. I do not think they have caused the build issues.“

So he is telling me about Curtis but it’s nothing to do with the problem I am trying to help him with?

After looking at Neil’s local changes, I saw that he had made changes to the build script which is basically one of my initial questions which he could have answered. I suppose I need to check each stage for tampering.

So I ask him what the error he got in order for him to start changing the build script. It shouldn’t be necessary; why would it work for everyone in the department but not Neil? He sends me a screenshot, and I noticed that the command prompt showed Isobel’s name as the local user.

How can he be logging in as Isobel? Have IT merged his account. Is this Curtis’ doing?

I asked him if he noticed he was logging in as Isobel. He says he took the screenshot from her documentation! He is a wind-up. He was getting the same error as her troubleshooting guide but decided to paste her screenshot to me.

I tell him to log in; then run a command. He runs the command without logging in. So I explained again, and I think he logged in but didn’t run the command. Absolute wind-up.

In the end I think we undid all his random changes to the build script and fixed the problem with the Azure Devops plugin, as documented in Isobel’s guide.

How Many Items To Return

There was a bug he was assigned to that looked like it could be simple on first glance. You have a list of items and the code was calling the LINQ method First(). The change could be that it should be FirstOrDefault if it could be an empty collection, or maybe the problem is that there really should be one item in the list and it is missing.

Without recreating the problem, we wouldn’t know what the fix should be.

Looking at the code, it passed in a list of items and possibly switched some depending on the config and certain checks. The method returned a list but was not clear at first glance what was returned. Is it all items? ones that have switched? any that have been modified?

He said it should be all items regardless if they were swapped.

Then when reading the code in this particular method where the crash occurs, why was it returning items when this swapping feature was enabled, but 0 if not?

Another developer, Dean, points out that in the previous method calls, there is another IF statement. When this swapping feature is on, an empty list is returned, and when it isn’t, the original drugs are returned. But then they are placed in a list called switchedItems.

Neil: “I understand it enough to know that this fix will work, but don’t understand it enough to refactor it”

He says his approach is to make the simplest change possible, not increase the scope and chance of introducing a new bug. I say that although that approach is generally good, in this case we see that the original developer had made a mistake in a previous method, and not returned the initial list of items. Then it looked like they had put in a hack to possibly try and work around it.

So I tell Neil that the original change was basically a hack – and the developer was attempting to do the simplest and quickest fix; but it has then made the code look confusing and still has a bug. Then Neil is continuing this mentality, by modifying the if statement to work in this additional scenario where we have found a bug, further contributing to the mess.

We discussed other scenarios and if those scenarios are impacted by this particular code. Then David says “so my change is fine then?”. Dean and I, in unison, say “NO!”. You need to fix the original method to return the initial list.

Not Hitting Breakpoints

There was another good example where he did ask for help but hadn’t recreated the issue at all, and didn’t adequately explain what point in the investigation he was stuck on.

The bug report was very poorly worded, but the general gist of the problem as I understood was – that there was a certain type of Request Task which contained a list of items. The user then tries to change the item to a Repeat order. Then it crashed triggering a dialogue.

His manager asked him if he had tried recreating the issue, and he said he has been “trying for a while“, but didn’t elaborate on it.

I gave him advice that sometimes you need to look where the crash occurs from the stack trace, then look at the method calls before that to see the initial UI call. Then work out which UI control triggers that.

The next day, he says he is still struggling, so I needed to help him.

I get set up. After I start investigating and getting sidetracked with other issues, he then says he is getting a crash creating the Tasks. What has he been doing for the last few hours?

I sorted his environment out, then asked him if he had found any suspicious code, and he said he hadn’t. I wasn’t sure if he meant he hadn’t even found any relevant code, or hadn’t identified what could be the problem, but I left him to it for a bit.

He sends me some method names which lead up to the method which was mainly from the stacktrace, and some were irrelevant. I pointed out that the line of code that was crashing was when a collection was empty, and reminded him the exact method he needs to look at.

A few hours later, he asked me if I had made any progress, and then said “there is a null being returned in the If statement”. So I remind him that null is fine, we are looking when the collection is empty. Empty or null are similar, but there’s a difference in the world of programming.

So we need to find what can return an empty collection. The returned list sometimes does stuff with the swapping feature so that made me think it could be that.

Neil 
Which dialog are you referring to when you say "in that dialog"?
Me 
In the Task
Neil  
Are you adding a task that appears in the To-Do list or Inbox?
Me 
Requests, not To-Do list
have you not got that far, or have I just confused you by giving you random bits of info

I don’t really get it because it sounded like he knew it was Requests, and then is now thinking it could just be some other type of task. Then when I explain where the link is that the user is clicking in the stack trace, he is like “oooooooh I haven’t got this far“.

It said Requests in the bug report. I told him to first work out what the user was clicking in the UI. But he hasn’t got far enough to know it was a Request nevermind what the user was clicking on.

As he was showing me debugging, it was hitting all kinds of breakpoints. He had like 50 and hadn’t been hitting them before because he wasn’t using the correct dialog. He had speculatively changed the method I had told him to look at as an experiment, but if he was never using the feature that called that code, then what is the point of experimenting by changing it?

He could have told me that he can’t hit his breakpoints at least. Either a problem in Visual Studio, or most likely; he isn’t in the correct feature of our program.

I don’t really understand how he can be a developer for 30 years and not understand how to work out a problem from the stacktrace. It’s not 100% possible due to a feature might needing configuration to be enabled, so you may still wonder how code is called, but at least you can deduce some information and come up with several scenarios then rule some out.

Conclusion

I always say it is hard to tell how good developers are, but there’s certain red flags that suggest they are bad. Obvious ones for me are not asking for help, not understanding basic debugging, showing a lack of interest in the project or code quality.

Teapot

I was looking through the list of HTTP status codes and saw a strange one.

Error 418 (I’m a teapot)!?

Google have a dedicated teapot page:

Error 418 (I’m a teapot)!?

If you hover over the teapot, his eyes open and he smiles. If you click it, it then animates.

It sounds like the origin was an April Fools prank.

RFC 2324 was written by Larry Masinter, who describes it as a satire, saying “This has a serious purpose – it identifies many of the ways in which HTTP has been extended inappropriately.” The wording of the protocol made it clear that it was not entirely serious; for example, it notes that “there is a strong, dark, rich requirement for a protocol designed espressoly for the brewing of coffee”.

wikipedia

Github copilot

We recently had staff from Github Copilot do a presentation on how their product can be useful to Software Developers. I found their answers to be a bit wishy-washy. I think it’s a really complex topic and having what I think were essentially sales managers trying to pitch something technical to us was a challenge. They didn’t have a full understanding of how it actually worked.

Someone asked a question to clarify if Copilot just looked at your open documents, or if it had the permission to see all the other files in your repository. Their answer was vague, along the lines of “it might do. Could come down to chance“.

For it to be effective, it really does need to look at your codebase to see what your product does, what features are already developed, and for bonus points, your coding style.

When it needs to suggest calling third-party code and installing additional libraries, does it understand that you may need to abide by a certain licence (pay some fee, or not use it in open-source etc)? and does it know that you may be limited to a certain version of it due to other dependencies? when features and the API (required parameters etc) can change drastically between versions, does Copilot understand that?

It’s probably the same scenario as what Wolfram Alpha were talking about when they came to our company to do a presentation on AI. They were emphasising how standard language models often suggest some text which reads like it makes sense, but it’s actually nonsense. They gave an example where it quoted a real journal from that country, stated the title of a chart that exists, quoted some figures and years – but the figures were fictional.

I saw a news article about how a lawyer presented some documentation to a judge about similar cases, but it turns out the lawyer had used ChatGPT and it had made up the case numbers and years.

The way those models work is that it knows some related words, and knows sentence structure, but the likes of ChatGPT doesn’t understand that something like that needs to be accurate and you can’t make stuff up. So Wolfram were saying their plugin can be combined with ChatGPT’s conversational structure to plug in actual figures to make accurate essays. TEAMWORK.

I would imagine there’s a good chance Copilot has exactly the same issue. It knows a bit of structure, slaps in the correct programming language, but it has no idea that it’s from a different library version that you aren’t using.

From what I have seen of Copilot, it is very impressive but does often give you code that doesn’t quite compile but gives you a good template and inspiration of how to progress.

In the past I have seen people blindly copy code from the internet, or just do what a colleague suggests without actually thinking about it. I think we are gonna be seeing this more from now on, but it’s gonna be the AI’s fault.

I am not against AI in programming because it can speed up development in certain tedious areas, but it always comes down to the idea that the best programmers are ones with a certain mindset of quality, and I think AI is gonna produce more developers with the wrong mindset because it’s about speed and cutting corners.

I’ve heard people suggest that the next wave of developers can be so dependent on AI, that they will be unable to come up with a solution when the AI doesn’t get it right.