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

Put the KVPs of identifiers and properties fields of resource shapes on their own lines #2377

Conversation

brandondahler
Copy link

@brandondahler brandondahler commented Aug 23, 2024

Background

What do these changes do?

Updates the formatting logic to put the key-value pairs of the identifiers and properties fields of resource shapes on individual lines instead of combining them onto a single line

Why are they important?

While the existing formatting works for simple single-identifier resources, resources with multiple identifiers quickly become unwieldy due to their identifiers being forced onto a single line.

For identifiers this is only an annoyance because most resources only have 1 or 2 identifiers. For properties, this can easily get out of hand because resources will generally have many resources.

For example, this is the current formatted version of a resource within a service I'm currently developing:

resource FooBar {
    identifiers: { fooId: FooId, FooBarId: FooBarId }
    properties: { previousFooBarId: FooBarId, fizz: String, buzz: String, baz: BazId, createdAt: Timestamp }
    create: CreateFooBar
    read: GetFooBar
    list: ListFooBar
}

My argument here is that the following format is more easy to parse as a reviewer:

resource FooBar {
    identifiers: {
        fooId: FooId
        FooBarId: FooBarId
    }
    properties: { 
        previousFooBarId: FooBarId
        fizz: String
        buzz: String
        baz: BazId
        createdAt: Timestamp 
    }
    create: CreateFooBar
    read: GetFooBar
    list: ListFooBar
}

Alternatives

An alternative I'm willing to look into implementing is updating the logic that currently forces all of the items of key-value pairs inside of objects into being smarter about when to place the items on a single line. A reasonable behavior in my opinion would be for it to keep values to a single line if there's only a single item, switching to the multi-line behavior when there's more than a single item.

Testing

  • Updated and improved corpus tests for the formatter to verify the updated behavior works as expected

Links


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@brandondahler brandondahler requested a review from a team as a code owner August 23, 2024 14:05
Copy link
Contributor

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

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

I'm okay with this, looks great! Thanks for the contribution!

@JordonPhillips JordonPhillips merged commit 0f64ac5 into smithy-lang:main Aug 23, 2024
13 checks passed
@brandondahler brandondahler deleted the features/IdentifierPropertyFormatting branch August 23, 2024 18:15
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.

How to get smithy-format to keep resource identifiers and properties on separate lines
2 participants