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

Fix clippy lints and run clippy in CI #29

Merged
merged 11 commits into from
Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ jobs:
- name: Check style
run: cargo fmt -- --check

clippy-lint:
runs-on: ubuntu-18.04
steps:
# actions/checkout@v2
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
- name: Run Clippy Lints
run: cargo clippy -- -D warnings

build-and-test:
runs-on: ${{ matrix.os }}
strategy:
Expand Down
4 changes: 4 additions & 0 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ suite runs cleanly and should remain clean.
You can **format the code** using `cargo fmt`. Make sure to run this before
pushing changes. The CI checks that the code is correctly formatted.

The https://github.com/rust-lang/rust-clippy[Clippy Linter] is used to help
keep the codebase efficient and idiomatic, and can be executed with
`cargo clippy`.

To **run the system:** there are two executables: the `oxide_controller` (which
is a real prototype backed by an in-memory store; this component manages an
Oxide fleet), and the `sled_agent` (which is a simulation of the component that
Expand Down
9 changes: 3 additions & 6 deletions api_identity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,9 @@ fn do_api_identity(item: TokenStream) -> Result<TokenStream, syn::Error> {
let name = &ast.ident;

if !match ast.fields {
Fields::Named(ref fields) => {
fields.named.iter().any(|field| match &field.ident {
Some(ident) if ident.to_string() == "identity" => true,
_ => false,
})
}
Fields::Named(ref fields) => fields.named.iter().any(
|field| matches!(&field.ident, Some(ident) if *ident == "identity"),
smklein marked this conversation as resolved.
Show resolved Hide resolved
),
_ => false,
} {
return Err(syn::Error::new_spanned(
Expand Down
6 changes: 3 additions & 3 deletions src/datastore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,9 +628,9 @@ impl ControlDataStore {
* TODO-cleanup this is only public because we haven't built Servers into the
* datastore yet so the controller needs this interface.
*/
pub fn collection_page<'a, 'b, KeyType, ValueType>(
search_tree: &'a BTreeMap<KeyType, Arc<ValueType>>,
pagparams: &'b DataPageParams<'_, KeyType>,
pub fn collection_page<KeyType, ValueType>(
search_tree: &BTreeMap<KeyType, Arc<ValueType>>,
pagparams: &DataPageParams<'_, KeyType>,
) -> ListResult<ValueType>
where
KeyType: std::cmp::Ord,
Expand Down
24 changes: 12 additions & 12 deletions src/http_pagination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ where
{
ApiPageSelector {
scan: scan_params.clone(),
last_seen: scan_params.marker_for_item(item).clone(),
last_seen: scan_params.marker_for_item(item),
}
}

Expand Down Expand Up @@ -184,10 +184,10 @@ where
* test the bulk of the logic without needing to cons up a Dropshot
* `RequestContext` just to get the limit.
*/
fn data_page_params_with_limit<'a, S>(
fn data_page_params_with_limit<S>(
limit: NonZeroUsize,
pag_params: &'a PaginationParams<S, ApiPageSelector<S, S::MarkerValue>>,
) -> Result<DataPageParams<'a, S::MarkerValue>, HttpError>
pag_params: &PaginationParams<S, ApiPageSelector<S, S::MarkerValue>>,
) -> Result<DataPageParams<S::MarkerValue>, HttpError>
where
S: ScanParams,
{
Expand Down Expand Up @@ -287,7 +287,7 @@ impl ScanParams for ApiScanById {
PaginationOrder::Ascending
}
fn marker_for_item<T: ApiObjectIdentity>(&self, item: &T) -> Uuid {
item.identity().id.clone()
item.identity().id
}
fn from_query(p: &ApiPaginatedById) -> Result<&Self, HttpError> {
Ok(match p.page {
Expand Down Expand Up @@ -384,7 +384,7 @@ impl ScanParams for ApiScanByNameOrId {
let identity = item.identity();
match pagination_field_for_scan_params(self) {
ApiPagField::Name => ApiNameOrIdMarker::Name(identity.name.clone()),
ApiPagField::Id => ApiNameOrIdMarker::Id(identity.id.clone()),
ApiPagField::Id => ApiNameOrIdMarker::Id(identity.id),
}
}

Expand Down Expand Up @@ -437,10 +437,10 @@ pub fn data_page_params_nameid_name<'a>(
data_page_params_nameid_name_limit(limit, pag_params)
}

fn data_page_params_nameid_name_limit<'a>(
fn data_page_params_nameid_name_limit(
limit: NonZeroUsize,
pag_params: &'a ApiPaginatedByNameOrId,
) -> Result<DataPageParams<'a, ApiName>, HttpError> {
pag_params: &ApiPaginatedByNameOrId,
) -> Result<DataPageParams<ApiName>, HttpError> {
let data_page = data_page_params_with_limit(limit, pag_params)?;
let direction = data_page.direction;
let marker = match data_page.marker {
Expand All @@ -467,10 +467,10 @@ pub fn data_page_params_nameid_id<'a>(
data_page_params_nameid_id_limit(limit, pag_params)
}

fn data_page_params_nameid_id_limit<'a>(
fn data_page_params_nameid_id_limit(
limit: NonZeroUsize,
pag_params: &'a ApiPaginatedByNameOrId,
) -> Result<DataPageParams<'a, Uuid>, HttpError> {
pag_params: &ApiPaginatedByNameOrId,
) -> Result<DataPageParams<Uuid>, HttpError> {
let data_page = data_page_params_with_limit(limit, pag_params)?;
let direction = data_page.direction;
let marker = match data_page.marker {
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
* it's expected that we'll have links to private items in the docs.
*/
#![allow(private_intra_doc_links)]
/*
* TODO(#32): Remove this exception once resolved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this TODO here. The issue should cover it. If somebody wants to know why it was added, they can see the notes in this PR.

*/
#![allow(clippy::field_reassign_with_default)]

mod api_error;
pub mod api_model;
Expand Down