Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change thresholds-error verb from breach to cross #3099

Merged
merged 1 commit into from
May 31, 2023

Conversation

MattDodsonEnglish
Copy link
Contributor

The error message "thresholds have been breached" sounded harshly to my ear. Normally I associate "breaches" with illegal acts, data exfiltrations, and military sieges.

I proposed changing the verb to "cross," which is the verb used by the docs. True, I believe I wrote those docs, so I'm biased. But I also checked the Corpus of Contemporary American English, and "cross" is by far the most common collocation (word that co-occurs) with "threshold".

image

The word "cross" appears much more frequently with "threshold"
than the word "breach" does.
@github-actions github-actions bot requested review from imiric and oleiade May 29, 2023 10:38
@codecov-commenter
Copy link

Codecov Report

Merging #3099 (799fc4e) into master (00c98f4) will increase coverage by 0.18%.
The diff coverage is 87.64%.

❗ Current head 799fc4e differs from pull request most recent head fecb5fa. Consider uploading reports for the commit fecb5fa to get more accurate results

@@            Coverage Diff             @@
##           master    #3099      +/-   ##
==========================================
+ Coverage   73.50%   73.69%   +0.18%     
==========================================
  Files         238      239       +1     
  Lines       18220    18258      +38     
==========================================
+ Hits        13393    13455      +62     
+ Misses       3956     3935      -21     
+ Partials      871      868       -3     
Flag Coverage Δ
ubuntu 73.69% <87.64%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
js/initcontext.go 88.88% <ø> (+8.88%) ⬆️
js/modules/gomodule.go 77.27% <ø> (ø)
js/modules/require_impl.go 76.47% <76.47%> (ø)
output/cloud/expv2/output.go 81.70% <85.10%> (+36.11%) ⬆️
js/bundle.go 85.09% <88.23%> (-0.15%) ⬇️
js/modules/resolution.go 90.32% <90.32%> (ø)
cmd/run.go 72.76% <100.00%> (ø)
js/modules/cjsmodule.go 86.95% <100.00%> (ø)
js/modulestest/runtime.go 100.00% <100.00%> (ø)
metrics/engine/engine.go 88.98% <100.00%> (ø)

... and 2 files with indirect coverage changes

Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, sounds good to me. 👍

I don't find "breach" unclear, but you're right that "cross" is less harsh.

@MattDodsonEnglish
Copy link
Contributor Author

MattDodsonEnglish commented May 30, 2023

I don't find "breach" unclear, but you're right that "cross" is less harsh.

Oh sorry, I was unclear myself. I didn't mean semantically harsh (though it is), but that breach is an unusual verb for threshold, so it seems a little out of place.

What's more, breach is just a more unusual word than cross in general, so the simplified error message should be easier to understand for a larger portion of the userbase, many (most?) of whom are ESL.

@imiric imiric added this to the v0.45.0 milestone May 31, 2023
@imiric imiric merged commit 0761ba8 into master May 31, 2023
@imiric imiric deleted the dodson/not-breached-but-crossed branch May 31, 2023 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants