Skip to content

Simplify rustfmt config and switch to stable rustfmt #242

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

Merged
merged 2 commits into from
Jan 11, 2022
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
16 changes: 3 additions & 13 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,12 @@ jobs:
runs-on: ubuntu-18.04
steps:
- uses: actions/checkout@230611dbd0eb52da1e1f4f7bc8bb0c3a339fc8b7
- uses: actions-rs/toolchain@88dc2356392166efad76775c878094f4e83ff746
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zephraph I think this change is correct -- we don't have to specify this action any more in this job, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this action was just to get access to any rust toolchain, and we needed:

  • "nightly-2021-03-25" here, and
  • "Whatever is in the rust-toolchain.toml" file below

But it seems like, based on the "checks" output, we actually happen to be downloading the version we need (specified in the rust-toolchain.toml) on the first call to cargo --version.

Checking my understanding: With this new change, are we just relying on whatever happens to be installed on Github's default ubuntu-18.04 image to bootstrap?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't believe we're relying on whatever happens to be installed because we have a rust-toolchain file in the repo.

Copy link
Collaborator Author

@davepacheco davepacheco Jan 10, 2022

Choose a reason for hiding this comment

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

To expand on that: the Actions environment, at least for Ubuntu 18.04, includes Rust 1.57. I believe this is updated pretty regularly when a new Rust release comes out. So this will likely already be the right thing when this runs. But if it's not (e.g., if they've updated to 1.58 and we have not yet updated our rust-toolchain file), then the Rust that is installed by default will honor the rust-toolchain file like it always does, installing the specified toolchain and running the rest of the build with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, that's what I meant by "bootstrap" in my comment above. Sounds reasonable, if we expect that'll always exist in the Actions environment across our supported test systems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that's right. It's documented to be present in the Actions environments for Ubuntu, Windows, and Mac. I would imagine it would be a pretty big breaking change if it were removed. If it were, we could add this action back.

We might also add the action back if we wanted to override rust-toolchain to do builds with nightly or beta or some other stable (e.g., a MSRV).

with:
toolchain: nightly-2021-03-25
default: false
components: rustfmt
- name: Report cargo version
run: cargo +nightly-2021-03-25 --version
run: cargo --version
- name: Report rustfmt version
run: cargo +nightly-2021-03-25 fmt -- --version
run: cargo fmt -- --version
- name: Check style
run: cargo +nightly-2021-03-25 fmt -- --check
run: cargo fmt -- --check

clippy-lint:
runs-on: ubuntu-18.04
Expand All @@ -44,11 +39,6 @@ jobs:
os: [ubuntu-18.04, windows-2019, macos-10.15]
steps:
- uses: actions/checkout@230611dbd0eb52da1e1f4f7bc8bb0c3a339fc8b7
# Temporary fork until https://github.com/actions-rs/toolchain/pull/209 is merged
- uses: oxidecomputer/actions-rs_toolchain@ad3f86084a8a5acf2c09cb691421b31cf8af7a36
with:
profile: minimal
override: true
- name: Report cargo version
run: cargo --version
- name: Report rustc version
Expand Down
8 changes: 0 additions & 8 deletions .vscode/settings.json

This file was deleted.

22 changes: 2 additions & 20 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,16 @@ You can build the documentation yourself with:
$ cargo +nightly doc
----


== Contributing

For maintainers (e.g., publishing new releases and managing dependabot), see link:./MAINTAINERS.adoc[MAINTAINERS.adoc].

=== Building and testing

You can **build and run the whole test suite** with `cargo test`. The test
suite runs cleanly and should remain clean.

=== Code formatting and lint

Dropshot works with stable Rust versions, but for consistency the code is
_formatted_ with a specific version of `rustfmt`. To contribute to Dropshot,
you will need to have installed the `nightly-2021-03-25` version of the Rust
toolchain:

----
$ rustup install nightly-2021-03-25
----

You can then **format the code** using `cargo +nightly-2021-03-25 fmt` or
`rustfmt +nightly-2021-03-25`. Make sure to run this before pushing changes.
The CI uses this version of `rustfmt` to check that the code is correctly
formatted.
You can format the code using `cargo fmt`. CI checks that code is correctly formatted.

https://github.com/rust-lang/rust-clippy[Clippy] is used to check for common code issues. (We disable the style checks.) You can run it with `cargo clippy`. CI will run clippy as well.

For maintainers (e.g., publishing new releases and managing dependabot), see link:./MAINTAINERS.adoc[MAINTAINERS.adoc].

== Configuration reference

Expand Down
9 changes: 3 additions & 6 deletions dropshot/examples/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down Expand Up @@ -82,9 +81,7 @@ impl ExampleContext {
* Return a new ExampleContext.
*/
pub fn new() -> ExampleContext {
ExampleContext {
counter: AtomicU64::new(0),
}
ExampleContext { counter: AtomicU64::new(0) }
}
}

Expand Down
9 changes: 3 additions & 6 deletions dropshot/examples/file_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand All @@ -51,9 +50,7 @@ async fn main() -> Result<(), String> {
/*
* Specify the directory we want to serve.
*/
let context = FileServerContext {
base: PathBuf::from("."),
};
let context = FileServerContext { base: PathBuf::from(".") };

/*
* Set up the server.
Expand Down
9 changes: 3 additions & 6 deletions dropshot/examples/https.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down Expand Up @@ -130,9 +129,7 @@ impl ExampleContext {
* Return a new ExampleContext.
*/
pub fn new() -> ExampleContext {
ExampleContext {
counter: AtomicU64::new(0),
}
ExampleContext { counter: AtomicU64::new(0) }
}
}

Expand Down
5 changes: 2 additions & 3 deletions dropshot/examples/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down
9 changes: 3 additions & 6 deletions dropshot/examples/module-basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down Expand Up @@ -73,9 +72,7 @@ impl ExampleContext {
* Return a new ExampleContext.
*/
pub fn new() -> ExampleContext {
ExampleContext {
counter: AtomicU64::new(0),
}
ExampleContext { counter: AtomicU64::new(0) }
}
}

Expand Down
9 changes: 3 additions & 6 deletions dropshot/examples/module-shared-context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ async fn main() -> Result<(), String> {
* For simplicity, we'll configure an "info"-level logger that writes to
* stderr assuming that it's a terminal.
*/
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Info,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info };
let log = config_logging
.to_logger("example-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down Expand Up @@ -102,9 +101,7 @@ impl ExampleContext {
* Return a new ExampleContext.
*/
pub fn new() -> ExampleContext {
ExampleContext {
counter: AtomicU64::new(0),
}
ExampleContext { counter: AtomicU64::new(0) }
}
}

Expand Down
17 changes: 5 additions & 12 deletions dropshot/examples/pagination-basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ async fn example_list_projects(
.map(|(_, project)| project.clone())
.collect()
}
WhichPage::Next(ProjectPage {
name: last_seen,
}) => {
WhichPage::Next(ProjectPage { name: last_seen }) => {
/* Return a list of the first "limit" projects after this name. */
tree.range((Bound::Excluded(last_seen.clone()), Bound::Unbounded))
.take(limit)
Expand All @@ -105,9 +103,7 @@ async fn example_list_projects(
Ok(HttpResponseOk(ResultsPage::new(
projects,
&EmptyScanParams {},
|p: &Project, _| ProjectPage {
name: p.name.clone(),
},
|p: &Project, _| ProjectPage { name: p.name.clone() },
)?))
}

Expand All @@ -126,9 +122,7 @@ async fn main() -> Result<(), String> {
let mut tree = BTreeMap::new();
for n in 1..1000 {
let name = format!("project{:03}", n);
let project = Project {
name: name.clone(),
};
let project = Project { name: name.clone() };
tree.insert(name, project);
}

Expand All @@ -140,9 +134,8 @@ async fn main() -> Result<(), String> {
bind_address: SocketAddr::from((Ipv4Addr::LOCALHOST, port)),
..Default::default()
};
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Debug,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug };
let log = config_logging
.to_logger("example-pagination-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down
51 changes: 21 additions & 30 deletions dropshot/examples/pagination-multiple-resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,27 +119,25 @@ fn page_selector<T: HasIdentity>(
scan_params: &ExScanParams,
) -> ExPageSelector {
match scan_params {
ExScanParams {
sort: ExSortMode::ByIdAscending,
} => ExPageSelector::Id(Ascending, *item.id()),
ExScanParams {
sort: ExSortMode::ByIdDescending,
} => ExPageSelector::Id(Descending, *item.id()),
ExScanParams {
sort: ExSortMode::ByNameAscending,
} => ExPageSelector::Name(Ascending, item.name().to_string()),
ExScanParams {
sort: ExSortMode::ByNameDescending,
} => ExPageSelector::Name(Descending, item.name().to_string()),
ExScanParams { sort: ExSortMode::ByIdAscending } => {
ExPageSelector::Id(Ascending, *item.id())
}
ExScanParams { sort: ExSortMode::ByIdDescending } => {
ExPageSelector::Id(Descending, *item.id())
}
ExScanParams { sort: ExSortMode::ByNameAscending } => {
ExPageSelector::Name(Ascending, item.name().to_string())
}
ExScanParams { sort: ExSortMode::ByNameDescending } => {
ExPageSelector::Name(Descending, item.name().to_string())
}
}
}

fn scan_params(p: &WhichPage<ExScanParams, ExPageSelector>) -> ExScanParams {
ExScanParams {
sort: match p {
WhichPage::First(ExScanParams {
sort,
}) => sort.clone(),
WhichPage::First(ExScanParams { sort }) => sort.clone(),

WhichPage::Next(ExPageSelector::Id(Ascending, ..)) => {
ExSortMode::ByIdAscending
Expand Down Expand Up @@ -297,9 +295,8 @@ async fn main() -> Result<(), String> {
bind_address: SocketAddr::from((Ipv4Addr::LOCALHOST, port)),
..Default::default()
};
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Debug,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug };
let log = config_logging
.to_logger("example-pagination-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand Down Expand Up @@ -346,26 +343,20 @@ impl DataCollection {
};
for n in 1..1000 {
let pname = format!("project{:03}", n);
let project = Arc::new(Project {
id: Uuid::new_v4(),
name: pname.clone(),
});
let project =
Arc::new(Project { id: Uuid::new_v4(), name: pname.clone() });
data.projects_by_name.insert(pname.clone(), Arc::clone(&project));
data.projects_by_id.insert(project.id, project);

let dname = format!("disk{:03}", n);
let disk = Arc::new(Disk {
id: Uuid::new_v4(),
name: dname.clone(),
});
let disk =
Arc::new(Disk { id: Uuid::new_v4(), name: dname.clone() });
data.disks_by_name.insert(dname.clone(), Arc::clone(&disk));
data.disks_by_id.insert(disk.id, disk);

let iname = format!("disk{:03}", n);
let instance = Arc::new(Instance {
id: Uuid::new_v4(),
name: iname.clone(),
});
let instance =
Arc::new(Instance { id: Uuid::new_v4(), name: iname.clone() });
data.instances_by_name.insert(iname.clone(), Arc::clone(&instance));
data.instances_by_id.insert(instance.id, instance);
}
Expand Down
13 changes: 4 additions & 9 deletions dropshot/examples/pagination-multiple-sorts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,7 @@ async fn example_list_projects(
let data = rqctx.context();
let scan_params = ProjectScanParams {
sort: match &pag_params.page {
WhichPage::First(ProjectScanParams {
sort,
}) => sort.clone(),
WhichPage::First(ProjectScanParams { sort }) => sort.clone(),

WhichPage::Next(ProjectScanPageSelector::Name(Ascending, ..)) => {
ProjectSort::ByNameAscending
Expand Down Expand Up @@ -313,9 +311,8 @@ async fn main() -> Result<(), String> {
bind_address: SocketAddr::from((Ipv4Addr::LOCALHOST, port)),
..Default::default()
};
let config_logging = ConfigLogging::StderrTerminal {
level: ConfigLoggingLevel::Debug,
};
let config_logging =
ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Debug };
let log = config_logging
.to_logger("example-pagination-basic")
.map_err(|error| format!("failed to create logger: {}", error))?;
Expand All @@ -341,9 +338,7 @@ fn print_example_requests(log: slog::Logger, addr: &SocketAddr) {
ProjectSort::ByMtimeDescending,
];
for mode in all_modes {
let to_print = ProjectScanParams {
sort: mode,
};
let to_print = ProjectScanParams { sort: mode };
let query_string = serde_urlencoded::to_string(to_print).unwrap();
let uri = Uri::builder()
.scheme("http")
Expand Down
4 changes: 1 addition & 3 deletions dropshot/examples/petstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ async fn find_pets_by_tags(
tags: None,
status: None,
};
tmp = FindByTagsScanParams {
tags: page_selector.tags.clone(),
};
tmp = FindByTagsScanParams { tags: page_selector.tags.clone() };
(vec![pet], &tmp)
}
};
Expand Down
Loading