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

remove single whitespace prefix from comments and docs #1385

Merged
merged 5 commits into from
Sep 5, 2022

Conversation

SkymanOne
Copy link
Contributor

Closes #613

Only a single white space is removed which comes under the assumption that the user uses the standard cargo formatting style.

Example of trimming

...
"docs": [
  "# Flips the current value of the Flipper's boolean.",
  "Let's try to use some code",
  "```rust",
  "fn main() {",
  "    \"Hello, World!\"",
  "}",
  "```"
],
...

@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2022

Codecov Report

Merging #1385 (7bc882f) into master (5755802) will increase coverage by 0.36%.
The diff coverage is 83.33%.

❗ Current head 7bc882f differs from pull request most recent head 17753c0. Consider uploading reports for the commit 17753c0 to get more accurate results

@@            Coverage Diff             @@
##           master    #1385      +/-   ##
==========================================
+ Coverage   71.65%   72.01%   +0.36%     
==========================================
  Files         177      176       -1     
  Lines        5937     5911      -26     
==========================================
+ Hits         4254     4257       +3     
+ Misses       1683     1654      -29     
Impacted Files Coverage Δ
crates/metadata/src/utils.rs 70.00% <75.00%> (+3.33%) ⬆️
crates/metadata/src/specs.rs 73.95% <100.00%> (ø)
crates/allocator/src/bump.rs

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -56,3 +57,11 @@ where

deserializer.deserialize_str(Visitor)
}

pub fn trim_extra_whitespace(item: &'static str) -> &'static str {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub fn trim_extra_whitespace(item: &'static str) -> &'static str {
pub fn trim_extra_whitespace(item: &str) -> &str {

Lifetimes can be elided

this.spec.docs = docs.into_iter().collect::<Vec<_>>();
this.spec.docs = docs
.into_iter()
.map(trim_extra_whitespace)
Copy link
Collaborator

@ascjones ascjones Sep 5, 2022

Choose a reason for hiding this comment

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

Good spot, could you update the trim_docs test: https://github.com/paritytech/ink/blob/47b1fa3291dd7551002e780ace31df4d80e8539d/crates/metadata/src/tests.rs#L180 (or add a new one) to cover this, and also to test the updated trimming logic?

@@ -56,3 +57,11 @@ where

deserializer.deserialize_str(Visitor)
}

pub fn trim_extra_whitespace(item: &str) -> &str {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also add a short comment to this public function

@SkymanOne SkymanOne merged commit 7ddf87f into master Sep 5, 2022
@SkymanOne SkymanOne deleted the gn-trim-whitespaces branch September 5, 2022 17:16
xermicus pushed a commit that referenced this pull request Sep 8, 2022
* remove single whitespace prefix from comments and docs

* format code to please CI

* elided lifetime + tests

* adds comments to tests

* add docs for public function
This was referenced Sep 20, 2022
@ascjones ascjones mentioned this pull request Feb 15, 2023
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