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

Stale code #220

Closed
pzmarzly opened this issue Jun 15, 2019 · 3 comments · Fixed by #215
Closed

Stale code #220

pzmarzly opened this issue Jun 15, 2019 · 3 comments · Fixed by #215

Comments

@pzmarzly
Copy link
Collaborator

After stumbling upon unlinked file during development in #215 , I created a script to find other unlinked files. Here are the results for current master:

Stale files (safe to remove):

    components/common/src/async_adapter.rs
    components/common/src/frame_codec.rs
    components/common/src/wait_spawner.rs
    components/funder/src/credit_calc.rs
    components/funder/src/freeze_guard_legacy.rs
    components/stctrl/src/bin/stverify.rs

freeze_guard_legacy.rs will be removed once #215 is merged. Please decide what to do with other files. I think it would be nice to have similar analysis for unused functions, but existing utilities (like warnalyzer) are not ready for codebases as large as offst yet.

@realcr
Copy link
Member

realcr commented Jun 15, 2019

@pzmarzly : Thanks for putting this analysis. I just looked at stale-rs, pretty cool idea! I have never heard of warnalyzer. Do you have any idea why won't it work for Offst? Maybe we can file it as an issue there?

About the stale files you mentioned, you can remove everything besides stverify.rs, which is an actual binary. I wonder why it was detected as unused, but stctrl.rs wasn't.

EDIT: I know why. Because stctrl/Cargo.toml contains:

[lib]
name = "stctrl"
path = "src/lib.rs"

[[bin]]
name = "stctrl"
path = "src/bin/stctrl.rs"

But stverify.rs is not included as a binary. Could you add it?
About the rest of the stuff: It's sad to see them go, but we are using git, so they will always be here with us somewhere (:

@pzmarzly
Copy link
Collaborator Author

warnalyzer does not support projects with multiple targets (binaries) or tests. I saw it announcement about a week ago on Reddit, it's a new project, so it needs time.

pzmarzly added a commit to pzmarzly/offst that referenced this issue Jun 15, 2019
@pzmarzly
Copy link
Collaborator Author

I pushed into #215 , as some changes were already done there (freeze_guard_legacy.rs).

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 a pull request may close this issue.

2 participants