Tuesday, February 27, 2024

"LGTM" isn't automatically a bad code review comment

What's the reason for doing a code review?


It's to check that the code does what it is supposed to and that the reviewer is happy to have it as part of the code base.


If the code changes look fine and the reviewer is happy, they shouldn't be expected or obliged to give (write) more feedback than is necessary.


What's not good is pointless comments or references to things that weren't changed as part of what is being reviewed.


A reviewer should not try to prove they've looked at the code by providing unnecessary or unnecessarily detailed feedback.

It's not a good use of time for the person doing the review.

Dealing with (responding to) those unnecessary comments is also not a good use of the time for the person who requested the review.


Writing something, even if it's a few characters (or an emoji) that indicates that the approval wasn't fully automated or done by accident is fine by me.

Of course, if all someone ever did was comment on code they're reviewing this way then that should raise different concerns.



Don't write more than you need to or for the sake of it.

Don't comment just to show you've looked at something.



Monday, February 26, 2024

Before you "add AI" to your software...

Is your software amazing?

Are there no usability issues?

Do the people using your software never have problems?

Are there "niggly" little issues that can be frustrating but have never been given enough priority to actually be fixed?

Do these things annoy, frustrate, disappoint, upset, or turn off users?


If your software (app/website/whatever) can't get the basics right, it might be tempting to "add AI" (in whatever form) to your app but the hype may not be worth it.

Yes, AI is powerful and can do amazing things, but if the people using your software see you failing to get the basics working correctly, how will they feel when you add more sophisticated features?

If you've demonstrated a failure to do simple things, will they trust you to do it he complex things correctly?

Especially if you can't explain how the new feature works or directly control it?


I'm not saying don't use AI. I'm asking that if you demonstrate to your customers that you can't do simpler things without issue, should you really be doing more complex things?


Sunday, February 25, 2024

Do you run tests before checking in?

If not, why not?

That the CI will (should) run them is not an excuse for you not to.

I think it's right - but haven't checked, is not professional or good enough.

Even worse than simply relying on the CI to run all the tests of the work you've done is asking for someone else to review the changes before the CI has even run the automated checks against it. 
How would you feel if I asked you to review some of the work I've done but haven't verified it's correct and doesn't break some other part of the system?

The same applies to linting or ensuring that code meets the defined standards and formats. You shouldn't be relying on something checking this after you've said you have finished.
You really should be using tooling that does this as you go.



Some test suites do take a very long time to run. Have you tried running only the tests related to the area you're working on?


If your test suite takes too long to run:

- Have you run the bits relevant to your changes?

- What work is being done (or planned) by you and your team to make the test suite run faster?



Why you shouldn't use LangVersion = latest

Unless you absolutely must.
Cross through code snippet: <PropertyGroup>    <LangVersion>latest</LangVersion> </PropertyGroup>

Never being one to be concerned about being controversial, this is why I don't think you should ever use the LangVersion of "latest" in your C# projects.


TLDR: "Latest" can mean different things on different machines and at different times. You avoid the potential for inconsistencies and unexpected behavior by not using it.


I've had the (let's call it) privilege of working as part of a certain distributed team on a large code base.

There was a global setting, hidden away somewhere, which set the LangVersion to latest. This was mostly fine until one member of the "team" updated the code to use some language features in version 10 on the day it was released. They committed the code, and other team members pulled down the changes. But now the other team members (who hadn't yet updated their tooling--it was, after all, only release day) were getting errors and they couldn't understand why code was suddenly breaking. The use of the new language features wasn't necessary, and the confusion and wasted time/effort trying to work out where the errors had come from could have been avoided if the actual number had been updated in the settings and they hadn't relied on a value that changes over time and from machine to machine. The rest of the team would still have had to update their tool, but at least they would have gotten a useful error message.


And then, there was another project I worked on.

We were using an open-source library and as part of some "clever" MSBuild task that they included in the package, it was doing a check for the LangVersion. At some point in the past, they needed to do a workaround for a specific version number and also added that workaround for "latest". In time, the underlying issue was fixed, and so when the package was used in a project that tried to use the "Latest" LangVersion, the package was trying to apply a fix that was no longer necessary and actually caused an exception. Yes, this was a pain to resolve. And yes, by "pain", I mean a long, frustrating waste of time and effort.


"Latest" may be useful if you're doing cutting-edge work that is only supported by that LangVersion setting. For all other cases, you should specify an actual number.



Of course, you're free to ignore these arguments. If you do, I'd love to know why.



Friday, February 23, 2024

If XAML has problems, why not just abandon it?

I often hear that XAML has problems, so you should use C# instead.

If you really strongly object to using XAML and can't be persuaded otherwise, feel free to only use C#. I'm not going to stop you. I'm more interested in you producing high-quality software that you can maintain and that provides value to people and they can easily use.

If you're not going to make a purely emotional decision, read on.

  • If you already have existing XAML code, rewriting it probably isn't the best use of your time.
  • XAML or C# is an artificial distinction. It was never the case that you were intended to only ever use XAML and not C# for the UI of anything but the most basic of applications. There are some things that are very hard in XAML, and there are some things that _should_ feel strange and unnatural when done in C#.
  • There are better ways to write XAML than the very basic ways shown in almost all instructions and training material.