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

style: improve formatting of string concatenation #62

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

ee7
Copy link

@ee7 ee7 commented Feb 28, 2024

The lack of spaces around & here made these harder to read.

My first reading of:

&"

is the beginning of a strformat format string, like:

import std/strformat

const msg = "hello"
doAssert &"{msg}\n" == "hello\n"

Let's write:

& "

to clarify that we're doing string concatenation.

Also reformat a little and change .toLower to .toLower() while we're here, since using parens to distinguish calls from field accesses seems to be our style.

@ee7 ee7 requested a review from miki725 February 28, 2024 12:23
Base automatically changed from ee7/timeout-s3client-stsclient to dev February 28, 2024 13:07
The lack of spaces around `&` made this harder to read.

My first reading of:

    &"

is the beginning of a strformat [1] format string, but my first reading
of

    & "

is string concatenation.

[1] https://nim-lang.org/docs/strformat.html
@ee7 ee7 force-pushed the ee7/refactor-newHttpClient-params branch from 88ccdae to 01e270e Compare February 28, 2024 13:09
@ee7 ee7 marked this pull request as ready for review February 28, 2024 13:11
@ee7 ee7 requested a review from viega as a code owner February 28, 2024 13:11
@ee7 ee7 changed the title refactor(awclient, s3client, stsclient): clarify newHttpClient params refactor(awclient, s3client, stsclient): clarify string concatenation Apr 2, 2024
@ee7 ee7 changed the title refactor(awclient, s3client, stsclient): clarify string concatenation style: use space around string concatenation operator Apr 2, 2024
@ee7
Copy link
Author

ee7 commented Apr 2, 2024

Ping @miki725 - could you review? A trivial style-only change that's been sitting for a while - I'd like to reduce the number of open PRs in nimutils.

We can consider formatting the code later when it won't cause merge conflicts for people, but let's just merge this since (I guess) it bothered me enough to create it.

Copy link
Contributor

@miki725 miki725 left a comment

Choose a reason for hiding this comment

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

Great. All of these came from third party library which had some wonky formatting...

@ee7 ee7 merged commit e8f1661 into dev Apr 8, 2024
2 checks passed
@ee7 ee7 deleted the ee7/refactor-newHttpClient-params branch April 8, 2024 15:18
@ee7 ee7 changed the title style: use space around string concatenation operator style: improve formatting of string concatenation Apr 8, 2024
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