Skip to content

Use int type name consistently#10579

Merged
amtoine merged 11 commits intonushell:mainfrom
sholderbach:error-integrity
Oct 3, 2023
Merged

Use int type name consistently#10579
amtoine merged 11 commits intonushell:mainfrom
sholderbach:error-integrity

Conversation

@sholderbach
Copy link
Copy Markdown
Member

@sholderbach sholderbach commented Oct 2, 2023

Description

When referring to the type use int consistently. Only when referring to the concept of integer numbers use integer.

  • Fix random integer to random int tests
  • Use int instead of integer in error messages
  • Use int type name in bits commands
  • Fix messages in for examples
  • Use int typename in into commands
  • Use int typename in rest of commands
  • Report errors in nu-protocol with int typename

Work for #10332

User-Facing Changes

User errorrs should now use int so you can easily find the necessary commands or type annotations.

Tests + Formatting

Only two tests found that needed updating

Copy link
Copy Markdown
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

ooh nice, thanks for working on this 🙏

i see there are still quite a few mentions to integer on the tip of this PR branch, is that expected and out of the scope of this PR? 😋
e.g.

crates/nu-command/src/generators/seq_date.rs
286:                    "integer value too large".to_string(),
287:                    "integer value too large".to_string(),

@amtoine
Copy link
Copy Markdown
Member

amtoine commented Oct 2, 2023

also, you're first bullet point above will supersed one of the commits of #10568, which is fine 😉

@sholderbach
Copy link
Copy Markdown
Member Author

i see there are still quite a few mentions to integer on the tip of this PR branch, is that expected and out of the scope of this PR? 😋 e.g.

crates/nu-command/src/generators/seq_date.rs
286:                    "integer value too large".to_string(),
287:                    "integer value too large".to_string(),

I skipped a bunch of places where it being a nushell type int was not the primary focus. (and the mathematical concept integer may be relevant)

For the example you found we could shorten that as well.

Copy link
Copy Markdown
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

i had a quick look at the remaining results from rg integer crates/**/*.rs and it looked mostly fine to me 👌

thanks for the increased consistency 🙏

@amtoine amtoine merged commit 7c1487e into nushell:main Oct 3, 2023
@sholderbach sholderbach deleted the error-integrity branch October 3, 2023 16:53
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description
When referring to the type use `int` consistently. Only when referring
to the concept of integer numbers use `integer`.

- Fix `random integer` to `random int` tests
  - Forgot in nushell#10520
- Use int instead of integer in error messages
- Use int type name in bits commands
- Fix messages in `for` examples
- Use int typename in `into` commands
- Use int typename in rest of commands
- Report errors in `nu-protocol` with int typename

Work for nushell#10332 

# User-Facing Changes
User errorrs should now use `int` so you can easily find the necessary
commands or type annotations.

# Tests + Formatting
Only two tests found that needed updating
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.

2 participants