Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Re-enable non_snake_case warnings?

25 views
Skip to first unread message

Josh Matthews

unread,
Jan 9, 2020, 4:33:54 PM1/9/20
to dev-...@lists.mozilla.org
After coming across an instance of a non-snake case variable name that
didn't trigger any compiler warnings, I discovered that we disabled that
warning for the script crate 5 years ago
(https://github.com/servo/servo/commit/dc86e8365495acc87b983a290bb7277a37a5247f#diff-7efb30b262e93375ff2af348d5a870f9R11).
Any objections to re-enabling it in Servo-specific code throughout the
project and fixing the existing instances?

Cheers,
Josh

Simon Sapin

unread,
Jan 9, 2020, 4:59:23 PM1/9/20
to dev-...@lists.mozilla.org
No objection, but fixing may be non-trivial. Changing that `allow` to `deny` produces:

error: aborting due to 368 previous errors

It looks like many of these names are generated from WebIDL, so this might require
codegen change (not just renames in source code).

--
Simon

Cheng You Bai

unread,
Jan 10, 2020, 1:39:29 AM1/10/20
to dev-...@lists.mozilla.org
No objection either! but, currently, there are some functions which will be
communicated with mozjs bindings are named in camel case.
Would it be good to keep them in camel case way? (ex.
*HostResolveImportedModule* for *SetModuleResolveHook*)
> _______________________________________________
> dev-servo mailing list
> dev-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
>

Josh Matthews

unread,
Jan 10, 2020, 10:23:03 AM1/10/20
to dev-...@lists.mozilla.org
If they are direct FFI bindings then we can't rename them, since those
are the actual exported symbols names from mozjs. If they are custom
glue code we've written, my preference is to keep the UpperCamelCase
format if they are only transparent wrappers around C++ methods like
https://github.com/servo/rust-mozjs/blob/437460db87ef3c10f34388fc565d590110ead571/src/jsglue.cpp#L1049-L1117
..
0 new messages