Skip to content

Clean-up for PR #4027#5395

Merged
dlang-bot merged 3 commits intodlang:masterfrom
JackStouffer:slides-clean-up
May 16, 2017
Merged

Clean-up for PR #4027#5395
dlang-bot merged 3 commits intodlang:masterfrom
JackStouffer:slides-clean-up

Conversation

@JackStouffer
Copy link
Contributor

@JackStouffer JackStouffer commented May 15, 2017

  • fixed some small use of over-applied attributes
  • fixed some error messages
  • renamed slides to slide

@JackStouffer JackStouffer added the Review:Trivial typos, formatting, comments label May 15, 2017
@JackStouffer JackStouffer requested a review from wilzbach May 15, 2017 14:49
@property auto front()
{
assert(!empty, "Attempting to access front on an empty range");
assert(!empty, "Attempting to access front on an empty slides");
Copy link
Member

Choose a reason for hiding this comment

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

Plural noun used as singular?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's awkward, but that is the name of the function that the user called and it follows the error message convention of the other structs in this package.

Copy link
Member

Choose a reason for hiding this comment

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

That suggests "slider" may be a better name. Has "slides" been released yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andralex Typically range construction functions are verbs, e.g. map, filter, padLeft so maybe slide would be better?

Copy link
Member

Choose a reason for hiding this comment

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

affirmative, slide is better

@JackStouffer JackStouffer changed the title Small clean-up for PR #4027 Clean-up for PR #4027 May 16, 2017
@JackStouffer JackStouffer removed the Review:Trivial typos, formatting, comments label May 16, 2017
@JackStouffer
Copy link
Contributor Author

@andralex Fixed


Params:
f = If `Yes.slidesWithLessElements` slides with fewer
f = If `Yes.slideWithLessElements` slide with fewer
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we also should do s/slideWithLessElements/slideWithFewerElements/g or better yet (because it's obvious from context) s/slideWithLessElements/withFewerElements/g

@JackStouffer
Copy link
Contributor Author

@andralex Done. Also fixed two small grammar issues in the return statement.

@andralex
Copy link
Member

cc @wilzbach to keep him in the know

@dlang-bot dlang-bot merged commit 4dab22e into dlang:master May 16, 2017

Params:
f = If `Yes.slidesWithLessElements` slides with fewer
f = If `Yes.withFewerElements` slide with fewer
Copy link
Member

@PetarKirov PetarKirov May 17, 2017

Choose a reason for hiding this comment

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

@JackStouffer JackStouffer deleted the slides-clean-up branch July 7, 2017 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants