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

Modify `script` to prevent further violations of snake_case #25547

Merged
merged 1 commit into from Jan 20, 2020

Conversation

@kunalmohan
Copy link
Collaborator

kunalmohan commented Jan 17, 2020

Remove #![allow(non_snake_case)] from script/lib.rs and add #[allow(non_snake_case)] at each instance of violation.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #25531 (GitHub issue number if applicable)
@highfive
Copy link

highfive commented Jan 17, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/request.rs, components/script/test.rs, components/script/dom/textencoder.rs, components/script/dom/rtcpeerconnectioniceevent.rs, components/script/dom/dompoint.rs and 126 more
  • @KiChjang: components/script/dom/request.rs, components/script/test.rs, components/script/dom/textencoder.rs, components/script/dom/rtcpeerconnectioniceevent.rs, components/script/dom/dompoint.rs and 126 more
@jdm
Copy link
Member

jdm commented Jan 17, 2020

error: variable `storeBindings` should have a snake case name
   --> components/script/dom/gpudevice.rs:253:17
    |
253 |         let mut storeBindings = HashSet::new();
    |                 ^^^^^^^^^^^^^ help: convert the identifier to snake case: `store_bindings`
    |
    = note: `-D non-snake-case` implied by `-D warnings`
error: variable `maxLimits` should have a snake case name
   --> components/script/dom/gpudevice.rs:258:13
    |
258 |         let maxLimits = MaxLimits {
    |             ^^^^^^^^^ help: convert the identifier to snake case: `max_limits`
error: aborting due to 2 previous errors

A couple more that you'll need to rebase to address. Otherwise I think this is a good choice :)

@@ -366,6 +366,7 @@ pub fn handle_get_browsing_context_id(
}

// https://w3c.github.io/webdriver/#dfn-center-point
#[allow(non_snake_case)]

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

This one can be fully snake-cased; the violations are just variable names.

@@ -900,7 +900,7 @@ impl StreamConsumer {

/// Implements the steps to compile webassembly response mentioned here
/// <https://webassembly.github.io/spec/web-api/#compile-a-potential-webassembly-response>
#[allow(unsafe_code)]
#[allow(unsafe_code, non_snake_case)]

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

I think this one can be fully snake cased; the only violation I see is the parameter name _mimeType.

@@ -349,6 +349,7 @@ pub fn load_whole_resource(
}

/// https://html.spec.whatwg.org/multipage/#create-a-potential-cors-request
#[allow(non_snake_case)]
pub(crate) fn create_a_potential_CORS_request(

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

I think we usually leave "cors" lowercase, and can snake_case this

@@ -393,6 +394,7 @@ pub fn normalize_value(value: ByteString) -> ByteString {
}
}

#[allow(non_snake_case)]
fn is_HTTP_whitespace(byte: u8) -> bool {

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

http could be lowercase here

@@ -62,6 +62,7 @@ impl BluetoothRemoteGATTCharacteristic {
}
}

#[allow(non_snake_case)]
pub fn new(

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

This can be snake-case, it's just the parameter instanceID

@@ -51,6 +51,7 @@ impl BluetoothRemoteGATTDescriptor {
}
}

#[allow(non_snake_case)]
pub fn new(

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

like the above comment, just the instanceID parameter

@@ -738,6 +741,7 @@ pub fn entries_to_matrix(entries: &[f64]) -> Fallible<(bool, Transform3D<f64>)>
}

// https://drafts.fxtf.org/geometry-1/#validate-and-fixup
#[allow(non_snake_case)]
pub fn dommatrixinit_to_matrix(dict: &DOMMatrixInit) -> Fallible<(bool, Transform3D<f64>)> {

This comment has been minimized.

Copy link
@pshaughn

pshaughn Jan 18, 2020

Member

I think this one's just the is2D local variable.

@pshaughn
Copy link
Member

pshaughn commented Jan 18, 2020

Oh, I think I misunderstood the purpose of this commit! Those comments are still worth having somewhere to look at later, but they're not necessarily for this pr in particular.

@kunalmohan
Copy link
Collaborator Author

kunalmohan commented Jan 18, 2020

I think I'll do it here itself. It shouldn't take much time at this stage.

@jdm
Copy link
Member

jdm commented Jan 19, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 19, 2020

📌 Commit f7db4b7 has been approved by jdm

@highfive highfive assigned jdm and unassigned SimonSapin Jan 19, 2020
@bors-servo
Copy link
Contributor

bors-servo commented Jan 19, 2020

Testing commit f7db4b7 with merge e2bc874...

bors-servo added a commit that referenced this pull request Jan 19, 2020
Modify `script` to prevent further violations of snake_case

<!-- Please describe your changes on the following line: -->
Remove `#![allow(non_snake_case)]` from `script/lib.rs` and add `#[allow(non_snake_case)]` at each instance of violation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25531  (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 19, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 20, 2020

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Jan 20, 2020

@bors-servo retry

bors-servo added a commit that referenced this pull request Jan 20, 2020
Modify `script` to prevent further violations of snake_case

<!-- Please describe your changes on the following line: -->
Remove `#![allow(non_snake_case)]` from `script/lib.rs` and add `#[allow(non_snake_case)]` at each instance of violation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25531  (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

Testing commit f7db4b7 with merge 28078bd...

@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Collaborator

CYBAI commented Jan 20, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

Testing commit f7db4b7 with merge 6b0e0a5...

bors-servo added a commit that referenced this pull request Jan 20, 2020
Modify `script` to prevent further violations of snake_case

<!-- Please describe your changes on the following line: -->
Remove `#![allow(non_snake_case)]` from `script/lib.rs` and add `#[allow(non_snake_case)]` at each instance of violation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25531  (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Collaborator

CYBAI commented Jan 20, 2020

@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

Testing commit f7db4b7 with merge 74d1f02...

bors-servo added a commit that referenced this pull request Jan 20, 2020
Modify `script` to prevent further violations of snake_case

<!-- Please describe your changes on the following line: -->
Remove `#![allow(non_snake_case)]` from `script/lib.rs` and add `#[allow(non_snake_case)]` at each instance of violation.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25531  (GitHub issue number if applicable)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 20, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 74d1f02 to master...

@bors-servo bors-servo merged commit f7db4b7 into servo:master Jan 20, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@kunalmohan kunalmohan deleted the kunalmohan:25531-SnakeCase branch Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.