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

Cleaning codebase #1886

Open
mrpalide opened this issue Oct 18, 2024 · 1 comment
Open

Cleaning codebase #1886

mrpalide opened this issue Oct 18, 2024 · 1 comment
Labels
enhancement New feature or request needs further investigation We should probably look further into this before concluding.

Comments

@mrpalide
Copy link
Contributor

Unfortunately, due to a series of urgent tasks and some service deployment issues, we've strayed from the main development path in all service codebases for quite some time now. This has resulted in a significant amount of technical debt, which includes:

  • Writing tests: Both new and old code lacks unit and integration tests.
  • Code cleanup: Reviewing, removing, and resolving commented-out code.
  • Addressing TODOs: Organizing and completing outstanding TODOs.

This issue affects all repositories and services to varying degrees.
Given the substantial nature of this task, I've created a separate ticket to allow for a collective decision on how to address it at our convenience.

@mrpalide mrpalide added enhancement New feature or request needs further investigation We should probably look further into this before concluding. labels Oct 18, 2024
@0pcom
Copy link
Collaborator

0pcom commented Nov 5, 2024

The first thing to do is more uniformly implement the CI tests across the different repos - including skycoin/skycoin

This would include making the Makefiles have similar directives across different repos, ensuring the latest version of test dependencies; i.e. golangci-lint & golang in the different repos, and where possible updating the code dependencies in go.mod & vendor dir to latest versions.

we should also try to ensure, as developers / contributors, that we have the latest version of golangci-lint & golang installed locally

The second thing is to resolve all CI errors from golangci-lint.

There are basically new errors on every new version of golangci-lint, so the technical debt in this regard isn't necessarily from any action that we take or changes being made to the codebase.

I don't disagree about code cleanup or tests; tests are good to have, but there are a few somewhat major issues I suggest we attempt to resolve first.

We are not that far from skywire being feature complete. The main issues are multihop routing not working and revisions to the skyfwd / skyrev which will allow for proxying (or reverse proxying) applications over skywire using transports.

It would be good to get multihop routing to work, but we need a better soltuion for concurrrency between tpd data and the transports visors have registered locally.

I suggest, and have made tickets regarding integrating or combining the uptime tracker with transport discovery ; and the same for integrating the uptime tracker with dmsg-disc. Basically eliminating the uptime tracker as a separate service.

with those changes to the transport discovery we should also collect the transport bandwidth and ping metrics via the transport discovery.

And if it's possible to resolve the multihop routing issue, we should do that as well.

After these things, I would support a thorough refactoring of the code basically as you have described.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs further investigation We should probably look further into this before concluding.
Projects
None yet
Development

No branches or pull requests

2 participants