Make methods static. #1313
Make methods static. #1313
Conversation
teymour-aldridge
force-pushed
the
implement-default
branch
from
2123186
to
c652d51
Compare
teymour-aldridge
force-pushed
the
implement-default
branch
from
dfe1feb
to
363a2a3
Compare
There was a problem hiding this comment.
Weird that codeowners pulled me in for a review here, but it looks good to me aside from a couple of tests that don't seem to be compiling at the moment:
examples/webgl/src/lib.rs:58:41
examples/webgl/src/lib.rs:134:37
Its ultimately not my decision about this change to static methods, but I am in favor of it.
|
@hgzimmerman Sorry about that; it was the result of a mess up over a rebase which meant that some commits which triggered you as a code owner were added on to this pull request – I've now removed them. |
mergify
bot
dismissed
hgzimmerman’s stale review
Pull request has been modified.
examples/npm_and_rest/src/ccxt.rs
Outdated
| @@ -12,7 +12,7 @@ extern "C" { | |||
| } | |||
|
|
|||
| impl CcxtService { | |||
| pub fn new() -> Self { | |||
| pub fn default() -> Self { | |||
There was a problem hiding this comment.
I think this was fine as it was, but if default is preferred, we should use the Default trait
There was a problem hiding this comment.
@jstarry Not exactly sure why I did this. It should be possible to have both the new constructor and implement Default as well (fewer breaking changes that way).
There was a problem hiding this comment.
Forgot that this was an example 😄. I think it's better to use Default that new here, though.
There was a problem hiding this comment.
Ok, do you mind removing the Option wrapping now that you dropped the #[derive(Default)]? I don't think it's needed after that
teymour-aldridge
force-pushed
the
implement-default
branch
from
dfa664b
to
fceef03
Compare
|
@teymour-aldridge The same thing could be done for |
|
I imagine so. |
Description
This makes a few methods which can be static, static.
Checklist:
./ci/run_stable_checks.sh