vapor-community / pagination

Simple Vapor 3 Pagination
MIT License
64 stars 15 forks source link

Rename key -> pageKey #4

Closed clayellis closed 6 years ago

clayellis commented 6 years ago

In order to increase clarity and consistency, I propose changing the parameter label key to pageKey in the paginate methods.

Motivation

The Swift API Design Guidelines urges us to "Include all the words needed to avoid ambiguity". By changing key to pageKey we remove any ambiguity about what key is — specifically that it is the page key in the query.

This also increases consistency throughout the code since everywhere else (see: Pagination.defaultPageKey) this key is referred to as the pageKey.

Breaking Change?

Potentially, but not drastic. Since the parameter is provided a default value, unless a user has specified the key parameter, there won't be any breaking changes. If, however, they have specified key the compiler can automatically suggest the renamed parameter label which makes upgrading trivial.

clayellis commented 6 years ago

We had talked on Discord about potentially changing instances of pageSize to match the new perPage wording. How do you feel about moving forward on that? Or is it unnecessary.

anthonycastelli commented 6 years ago

Yeah, I say change the pageSize to better match the style we are after here. Definitely makes more sense to keep everything matching and organized that pertains to each other.

vkill commented 6 years ago

+1

The page and per like the kaminari

anthonycastelli commented 6 years ago

I've gone ahead and merged this. 👍

clayellis commented 6 years ago

Awesome, I'll open another PR with the other wording changes.

anthonycastelli commented 6 years ago

Swiftlint was being a pain in the ass so had to fix all of that. But sounds good.