27 comments

[ 3.3 ms ] story [ 69.2 ms ] thread
I like the refactor bot idea.

I'd be a little nervous to lean on Copilot to refactor, but I'd love a bot that just implements the changes that IntelliJ can already recommend to me.

A bot that did other logically straightforward refactors (like extracting mapper methods from big functions) would also be nice.

Yeah the Copilot reference came out of nowhere. Given how important Copilot is to Github and how it's been getting trashed pretty hard, this article reads like some lowkey astroturfing for Copilot.
You'll want to keep an eye on Qodana[0] which when combined with ReviewDog[1] (and some glue script since Qodana has its own snowflake output JSON that RD doesn't read natively; engineering!) can offer suggested changes on MR/PR platforms which support such a thing [2]

I have the first two working together but not the last part yet, and Qodana is for sure a moving target but is what I've been praying for them to do for years now

0: https://github.com/JetBrains/Qodana/blob/2021.2/topics/getti...

1: https://github.com/reviewdog/reviewdog

2: https://docs.gitlab.com/ee/user/project/merge_requests/revie...

The refactor bot idea seems like it'd cause a lot of problems. If you're asked to review a PR that has a bunch of formatting issues, you now need to wait for the "prettier" bot to run and fix it. And if the PR author has to make additional changes, they could accidentally overwrite the commit made by the bot if they use "git push --force", which I've seen happen several times with Github's "suggested change" feature (you can prevent that by using "git push --force-with-lease" instead, but most devs are unaware of that IME).

An alternative solution to the problem with Prettier and other tools being closely tied to your editor/IDE is to adopt editor-agnostic formats like https://editorconfig.org/ .

I am guessing this applies more for javascript tooling

In Java, for 1 we use Sonar which is a very mature tool for code quality and for 2,IntelliJ does refactoring very well.

Looking at code climate, it looks like it was inspired by Sonar but I may be wrong

Sonarlint is propbably the best all around linter for every language that I’ve used.
Usually my biggest two wishes that seem to never be able to be satisfied is

  Ability to connect a debugger that sanely steps through code without getting lost in a library maze

  Get a useful stacktrace that's not just lines of framework wrapper garbage and then an elipses.
Also a few more, which I can usually get satisfied:

1 Don't fail silently, ever. Use UTC time in the log, I shouldn't have to say this.

2. Don't replace the default logging mechanism. Your wrapper version with the fancy colors isn't better. It just blocks the versatility and functionality of the actual language logging feature written by the language team who are far more competent than you. Stop that, you don't know what you're doing.

3 Inherit the environment in the process forks. Don't make me manually go in and pass things down.

4 Please Stop writing fancy error handlers. They often have bugs so the generated error will just be from a bug in the error handler thus hiding the original defect. It's a defect, all bets are off.

When they are working right they're frequently HTML/CSS/JS which is terrible for testing, statistics, and logging. Also they won't do the lower level exception throwing of the language that allows it to be caught by a debugger so the tooling breaks. You don't need to do this. It only makes things suck. Stop.

Alright that was cathartic

> Ability to connect a debugger that sanely steps through code without getting lost in a library maze

Depends on your environment? I use WinDbg and it does pretty well when symbols are loaded. User-Kernel jumps are still a bit of pita, but breakpoints work

I like how you pull out the best debugger ever made out of your back pocket. Yes, windbg deserves shrines and holidays. After switching away from fulltime windows dev about 13 years ago, it's one of my fondest memories of how amazing windows can actually be.
sanely steps through code without getting lost in a library maze

I'm not completely sure what you mean here? Debugger not stepping into 3rd party code? The debugger cannot know what you consider to be a maze and what not: are you developing said library? Then you want that code. You want only your code? Then you'll have to tell the debugger which is yours. Example implementation is debugging C or C++ with Visual Studio which has regex-based 'do not step into'.

Get a useful stacktrace that's not just lines of framework wrapper garbage and then an elipses.

Isn't this essentially the same as previous point? You want a relevant stacktrace, i.e. without 'getting lost in a library maze' so you'd need to filter. Don't know if that is implemented somewhere. I personally haven't encountered much need for that though: stacktraces contain the origin of the call (shared library or for Python etc the module path) so glancing over it I skip the 3rd party bits automatically.

In Java and c# why have we been typing pseudo-URLs (com.google etc) all these years if they aren’t going to be used for anything?
Sometimes you can't attach a debugger at all! I've dealt with plenty of things that will do some strange kind of fork/exec magic and shed all the debug information in the process.

And other things that will only show some small useless snippets of the stack. You pass a bunch of parameters to tell it to show more, but then invariably you find the code is using a custom logger that's cutting the thing off or some part of a library resets all your settings or the forked processes don't inherit everything, it almost never just does what you tell it to, there's something else in there ignoring it.

I wish I could have the opportunity to even get to that problem half the time

> Your wrapper version with the fancy colors isn't better.

makes me look over my shoulder.

> 3 Inherit the environment in the process forks. Don't make me manually go in and pass things down.

I do think this one has saved me a number of times though; being explicit. I find people polluting the namespace somewhat often; and it's more of an "don't do that" not "shit everything's broken"

I'm thinking of some project I'm working on where you pass in parameters and environment variables and it completely utterly ignores it.

It took me a while to figure it out. To start up the web server it calls some thread "provider" which calls a thread managing library manager, which calls yet another thread managing library which then forks some threads. Of course documentation is non-existent on most of it.

Within this garbled mess their was literally no way to say "I want to debug this as well" by passing down an environment variable.

I had to fork the various dependencies to pass down that. It's not a matter of well documented thoughtful code, that's not the issue. It's spaghetti dreams of teenage imaginations; an infrastructure of sandcastles ravaged by the oceans of time.

> Get a useful stacktrace that's not just lines of framework wrapper garbage and then an elipses.

This is the biggest pet peeve of mine. Over the years I've spent too much time ensuring I can get a stack trace *WITH* line numbers. A stack trace without line numbers makes support much more difficult.

For .NET why I need to generate and include .PDB files just to get line numbers I dunno. I've never ever used a .PDB for anything but stack trace line numbers. I'd bet that I'm not alone in this. Why the runtime could include them in the DLL (ok driven by a switch) I don't know.

And for JS those .MAP files can turn out to be larger than the original source files. Which is a bit mind boggling. And don't get me started on .ts -> js -> bundled to get the map files right. And the JS client libraries to handle unexpected exception capture and logging. And the server side parsing of the client side error and map files to get back to an original error line number in a .TS file.

It's downright shameful that the most assistance a runtime can provide is a stack trace without line numbers.

Don't replace the default logging mechanism. Your wrapper version with the fancy colors isn't better.

Disagree pretty strongly with this one. Colors make changes in the data pop instantly. Colors are for humans. Logs are for humans. Use structured data if you need computers to read it. Send that data to ELK stack. Also color is very easy to strip with a regex.

Though colors should be a logformatter on the output side, a wrapper or subclass of the system logger can still be useful for collecting context data that the system logger can't collect on its own.

The complaint wasn't color. Take laravel circa 2012 or so. All errors were expressed as like 80k of minified HTML so if you got an error in a unit test you better have a browser nearby to see it.

Some other systems will try to be helpful and traverse the object and display it on the screen. I've seen some that don't understand self linking datastructures and just give you a stack exhaustion whenever dealing with an error involving one because they try to infinitely recurse. Others, trying to "fix" this problem that's been easily fixable since the 60s with pretty simple algorithms, will instead only show a depth of maybe 2 or 3 levels and then you need to do a bunch of tricks to get a useful object

Other will not give thread information so you just get mystery errors without any way to attribute them.

Others will race condition the log and give you a confusing mess of partial data mashed together.

And yet others will just fall back to some generic "an error occurred" with no further information whatsoever. Still others will think you have no interest in strings over some random amount divisible by ten, say 50 characters long and just truncate them with no way around it.

And some you are expecting a longjump or a trap or an interrupt but instead it gets intercepted for the pretty logging and thus your tooling didn't see it and the faults go undetected

Then there's the ones that don't respect the call stack limit you gave the command line or the verbosity setting or things like polluting the call stack with layers of the error handlers.

There's one where the default logger synced on write so if the system crashed I'd get the last message, but the fancy logger didn't so I'd just get a bunch of corruption. It was just great!

They're generally speaking terribly done and I routinely have to disable them. It's just a clown car.

You probably have only worked on gifted perfect projects by flawless programmers, and I congratulate you!

I've dealt with mostly confused incompetent people who think they are really clever and just destroy things

> My projects typically use two tools that statically analyze code: linting, and tests.

Erm. Tests aren't static analysis.

I don't see the problem with running formatters in CI and I don't see how it could cause "chaos". The first stage in my CI pipelines is a QA check which includes running linters, formatters etc. It's up to each developer to make sure it passes and this can easily be achieved by installing the pre-commit hooks in their environments.

I do chuckle a little bit when I think about how all the linters and formatters and bundlers and transpilers and braindead test suites that have agglomerated onto JS development are effectively just a bad compiler with extra steps.
JS is the biggest local maxima for this sort of thing that the world has ever seen.
I strongly disagree with "bot refactors". Automated or semi-automated refactoring is great, but it's best run offline on a developer's machine.

Even if the system to do that works perfectly, you've still added more noise to the commit history, and in my opinion it still doesn't have any real advantages over doing it locally.

Our system for formatting code (actually similar to the author's current system):

1. The developer can choose whether or not to do auto-format on save.

2. The developer can run a single command in the repo to format all staged files. (Formatting all files in the repo is too slow).

3. A pre-commit hook checks that all changed files are correctly formatted.

4. On CI we check that all files are correctly formatted (in case any of the above didn't happen on the developer's machine).

It takes some effort to setup the system initially, but from then on it works really well. You get these advantages:

1. Formatting (or making any other change to a file) is always an explicit action.

2. The developer can review all changes before committing.

3. Every step of the process is easy to debug if something goes wrong (which is very rare).

4. There is minimal overhead to applying the auto-formatting.

5. Developers still have freedom in their personal workflows.

One example I've seen of a bot automatically creating PRs is dependabot to automatically update dependencies, but in my opinion it adds way more noise than value. Running a command locally to do the updates is just as easy, without any of the noise.

Is it too much to ask to a contributor to make contributions that match the code style guidelines? No arguments, if the code doesn't pass code style guidelines it's rejected. Also, normalize (interactive) rebase to pretend the code was perfect when written - "fix" and "style" commits should simply never make it into the main branch.
>One example I've seen of a bot automatically creating PRs is dependabot to automatically update dependencies, but in my opinion it adds way more noise than value. Running a command locally to do the updates is just as easy, without any of the noise.

It's just that you need to set up some kind of recurring reminder to do that kind of work, which ends up also being noise, and the work is a chore, nothing more. IMO the noise of a bot emailing you a ready-to-merge PR (usually 1 click) is better than the noise of a calendar event with at least 3 manual steps.

A simple vscode addon that puts functions in alphabetical order, if they can be.
IJ supports topological sorting which I think is much better, especially if combined with sane visibility ordering such that the most important are at the top

As as asterisk to the alphabetical order suggestion, for Java (and maybe others) it would separate the get, is, and set method pairs from one another, making read only properties harder to spot

> Nonbinary code quality

At a tool level, yeah that should be configurable.

GOLANG, I COMMENTED OUT ONE LINE. YOU KNOW HOW TO COMPILE WITH AN EXTRA IMPORT JUST F---ING DO IT.

But at the commit/merge request level, either the changes are merged or they aren't. They are either mergable or not mergable.

You can decide what things are errors and what are not, but printing tons of warnings is a great way to get ignored.