Skip to content

Conversation

@ee7
Copy link
Member

@ee7 ee7 commented Apr 11, 2021

I don't think this PR is a strict improvement, but I think overall it makes the spec more readable. The reader of

non-empty, non-blank string
non-empty, non-blank, lowercased string using kebab-case

will no longer ask questions like:

  • "do we allow a whitespace-only string somewhere?"
  • "is there a kebab-case string somewhere that's allowed to contain uppercase?"

I don't love the phrase "non-trivial string". Can we think of something better?

ee7 added 2 commits April 11, 2021 11:30
This also resolves the issue that the below rule did not previously
specify "non-blank":
> The `"exercises.practice[].practices"` values must be non-empty, lowercased strings using kebab-case
@iHiD
Copy link
Member

iHiD commented Apr 11, 2021

Thanks for this.

Could we use non-blank? As we define it, I think that's fine. (and also the definition you use is what Rails uses for non-blank too, so that fits with the consumer)

Copy link
Member

@iHiD iHiD left a comment

Choose a reason for hiding this comment

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

Looks good to me! I'll get Erik to confirm he's happy before merging :)

@ee7
Copy link
Member Author

ee7 commented Apr 13, 2021

@iHiD Can we merge this? I'll base another couple of PRs on top of these changes.

@iHiD
Copy link
Member

iHiD commented Apr 13, 2021

Certainly.

@iHiD iHiD merged commit 5309f55 into exercism:main Apr 13, 2021
@ee7 ee7 deleted the lint-strings branch April 13, 2021 15:38
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.

3 participants