5 ms·

The conclusion is the key takeaway. If you're finding spelling errors post-commit, it is too late to be directly actionable. Automatically-generated pull reques

The conclusion is the key takeaway. If you're finding spelling errors post-commit, it is too late to be directly actionable. Automatically-generated pull requests are just layering on more complications without addressing the core reality: If you can run a linter locally, before you commit, it is trivial to fix the problems where they stand. The same goes for many things that might be part of your CI pipeline today.

catillac 5y ago
Yeah this seems like something better as a pre commit check, or as a run on save, the same way I might run go lint or gofmt, not after the code is in the repo but as I’m saving. The “not talking to the users” mistake can be super fatal in products like this.
epage 5y ago
I'd phrase it slightly differently. We can and should fix things in response to CI failures. The problem is more so the cost/value trade off for developers. The lower the perceived value is, the more good will that will be lost with developers for having to do another iteration to fix them. Integrating these into the developers workflow as early as possible, like rustfmt or gofmt, lowers the cost and developers are less likely to have a problem with it. At this point, the main value of having something in CI is either for the reviewers to give them a sense of things, like coverage or to help enforce people are using those tools, which means it should ail CI.
lol768 5y ago
I don't really follow this. Ideally, the spelling errors should fail the build and ordinarily I wouldn't expect PRs to be able to be merged with failing CI checks. Presumably it's possible to suppress false positive typo reports too and flesh out a per-project dictionary to help all other developers?
- stefan_ 5y ago
  That is just fucking annoying for no benefit. Don't send me a "CI failed" email and make me click down into 4 different deep levels of pages to find out it failed because of a typo. Real text customers see lives in various i18n tools and you know what, they can tell you about typos as you make them!
bredren 5y ago
This is something Pycharm catches as a warning in a manual inspection or automatically prior to a commit. And while nice, the inspections often catch other more important code issues. These are also generally highlighted with an indicator of some sort to catch as you’re working on the code. Are pre-commit inspections / visual indicators not generally a part of most dev workflows? I would think spelling would be a part of the most basic text editor / ide at this point.
- Gibbon1 5y ago
  As someone whose spelling is generally subpar having continuous spelling checking in my editor is great. Best would be if it's integrated with a Language Server Protocol. For instance it would be nice to flag misspelled variable, function, and class names. And only flag them where they are defined. And not flag variable names in comments.
robertlagrant 5y ago
I'd say the way to do it is to have a --check option which fails if things are misspelled. That can be run in CI to catch problems, and leaves the developer to fix it in their own way (pre commit hook/IDE integration/command line tool/ability to spell).

catillac 5y ago

Yeah this seems like something better as a pre commit check, or as a run on save, the same way I might run go lint or gofmt, not after the code is in the repo but as I’m saving. The “not talking to the users” mistake can be super fatal in products like this.

epage 5y ago

I'd phrase it slightly differently. We can and should fix things in response to CI failures. The problem is more so the cost/value trade off for developers. The lower the perceived value is, the more good will that will be lost with developers for having to do another iteration to fix them. Integrating these into the developers workflow as early as possible, like rustfmt or gofmt, lowers the cost and developers are less likely to have a problem with it. At this point, the main value of having something in CI is either for the reviewers to give them a sense of things, like coverage or to help enforce people are using those tools, which means it should ail CI.

lol768 5y ago

I don't really follow this. Ideally, the spelling errors should fail the build and ordinarily I wouldn't expect PRs to be able to be merged with failing CI checks. Presumably it's possible to suppress false positive typo reports too and flesh out a per-project dictionary to help all other developers?

stefan_ 5y ago
That is just fucking annoying for no benefit. Don't send me a "CI failed" email and make me click down into 4 different deep levels of pages to find out it failed because of a typo. Real text customers see lives in various i18n tools and you know what, they can tell you about typos as you make them!

bredren 5y ago

This is something Pycharm catches as a warning in a manual inspection or automatically prior to a commit. And while nice, the inspections often catch other more important code issues. These are also generally highlighted with an indicator of some sort to catch as you’re working on the code. Are pre-commit inspections / visual indicators not generally a part of most dev workflows? I would think spelling would be a part of the most basic text editor / ide at this point.

Gibbon1 5y ago
As someone whose spelling is generally subpar having continuous spelling checking in my editor is great. Best would be if it's integrated with a Language Server Protocol. For instance it would be nice to flag misspelled variable, function, and class names. And only flag them where they are defined. And not flag variable names in comments.

robertlagrant 5y ago

I'd say the way to do it is to have a --check option which fails if things are misspelled. That can be run in CI to catch problems, and leaves the developer to fix it in their own way (pre commit hook/IDE integration/command line tool/ability to spell).