Skip to content

Conversation

@lumtis
Copy link
Contributor

@lumtis lumtis commented Jun 9, 2021

  • Create a package for name parsing
  • Support different name convention when parsing component name or field name from CLI
  • Generate the appropriate name convention in the code from the context (kebab case for commands for ex)
  • Create a package for fields parsing

For names (component or field) we will support only alpha characters, hyphen or underscore. This is open for discussion.

Maybe we could prevent using underscore since snake case is never used in the code but we should support hyphen since this is more convient for message and query scaffolding

@fadeev @ilgooz I did the implementation for the message scaffold as an example. I will wait for your feedback before moving forward

@lumtis lumtis requested review from fadeev and ilgooz June 9, 2021 09:32
@fadeev
Copy link
Contributor

fadeev commented Jun 9, 2021

When it's ready for review, this should work, right?

starport message create_post comment-id bodyText CommentLink Avatar_Image

Or is it too much? 🙈

@lumtis
Copy link
Contributor Author

lumtis commented Jun 9, 2021

Yes.

Not in this example because only the message name is parsed

@lumtis lumtis marked this pull request as ready for review June 11, 2021 09:36
Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Looks very good!

},
},
}
cases.expected[0].Name, _ = multiformatname.NewMultiFormatName("foo")
Copy link
Member

Choose a reason for hiding this comment

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

can we refactor this to execute each test over a loop like a table test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this part a bit cleaner but I'm not sure it it what you meant. This is already a table test but we have a single test that match an expected with an actual array

@lumtis lumtis requested a review from ilgooz June 14, 2021 11:24
fadeev
fadeev previously approved these changes Jun 14, 2021
Copy link
Contributor

@fadeev fadeev left a comment

Choose a reason for hiding this comment

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

Brilliant! One question I have that may be better addressed in a different PR is, don't we accept numbers in field names? Right now it's telling me "name cannot contain 5", but I'm not sure why.

@lumtis
Copy link
Contributor Author

lumtis commented Jun 15, 2021

Brilliant! One question I have that may be better addressed in a different PR is, don't we accept numbers in field names? Right now it's telling me "name cannot contain 5", but I'm not sure why.

No I can actually add this, we just need to prevent having a number as a prefix

Copy link
Contributor

@fadeev fadeev left a comment

Choose a reason for hiding this comment

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

🔥

@lumtis lumtis merged commit 5892648 into develop Jun 15, 2021
@lumtis lumtis deleted the feat/cases-support branch June 15, 2021 12:56
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.

Scaffolder: manage case conventions (camelCase kabab-case) for component names

4 participants