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

Development Debts #186

Open
jaoleal opened this issue Jul 3, 2024 · 7 comments
Open

Development Debts #186

jaoleal opened this issue Jul 3, 2024 · 7 comments
Labels
chore Cleaning, refactoring, reducing complexity code quality Generally improves code readability and maintainability dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@jaoleal
Copy link
Contributor

jaoleal commented Jul 3, 2024

After #176 i was alarmed by some development debt, which include:

  • Grammatical doc errors(discovered by typos.)
  • Incompatible Versions, such as
    • found crate miniscript compiled by an incompatible version of rustc
    • error: cannot determine resolution for the macro json
    • cannot find attribute serde in this scope
      all that captured by nightly clippy that ran by pre-commit and i wonder if that can be considered a bug since nightly is required for running fmt...

A proper fix to it is changing Actions(which enforce the use of nightly) and Pre-commit(Used so we do not have a surprise while being checked by Actions when pushing) to share the same version and behavior.

Dealing with nigthly directly is rough since is too unstable(Change every day), should we consider the use of beta for fmt ?
If nightly fmt is really required Ill mention the way that rust-bitcoin deal with it by reading their /.github files and scripts.

@jaoleal
Copy link
Contributor Author

jaoleal commented Jul 3, 2024

I think this can be tagged as Good First Issue and Improvement.

@Davidson-Souza Davidson-Souza added documentation Improvements or additions to documentation good first issue Good for newcomers dependencies Pull requests that update a dependency file chore Cleaning, refactoring, reducing complexity code quality Generally improves code readability and maintainability labels Jul 8, 2024
@Davidson-Souza Davidson-Souza added this to the v0.6.0 milestone Jul 8, 2024
@Davidson-Souza
Copy link
Collaborator

Added some tags. Do you want it to be assigned to you?

@jaoleal
Copy link
Contributor Author

jaoleal commented Jul 9, 2024

Would be nice! But i'd have some other priorities rn

@lucasbalieiro
Copy link

Hello, everyone!

I'm Lucas, and while I'm not a Rust programmer by trade, I can to contribute to the project by tackling some of the minor but impactful tasks.

Specifically, I’d like to address the pre-commit issue mentioned in relation to nightly clippy and formatting checks.

Before diving in, I wanted to ask if there’s already a proposed solution or direction for the pre-commit setup. Since this project is in Rust, using a Python-based pre-commit setup introduces an additional dependency, which might not align well with the project's ecosystem.

After exploring potential solutions, here’s what I have in mind:

  1. Using cargo-husky: This Rust-centric approach integrates directly with Cargo, which aligns better with the ecosystem. It simplifies the setup and avoids introducing Python dependencies.

  2. Custom Git Hook: This seems like the most flexible and reliable option. It would allow us to replicate the linting process currently used in GitHub Actions (e.g., using rustfmt directly) without adding extra dependencies. A custom hook can ensure consistent behavior between local checks and CI/CD workflows.

I’m leaning toward the second approach for its simplicity and customizability, but I’d love to hear your thoughts before proceeding. If it works for the team, I’m happy to take ownership of this issue and implement the agreed solution and leave your brains to attack more important issues in this project

@jaoleal
Copy link
Contributor Author

jaoleal commented Nov 21, 2024

Before diving in, I wanted to ask if there’s already a proposed solution or direction for the pre-commit setup. Since this project is in Rust, using a Python-based pre-commit setup introduces an additional dependency, which might not align well with the project's ecosystem.

That would be true only if the pre-commit was doing things differently from our github actions. Which is not the case anymore.

See #283

Custom Git Hook: This seems like the most flexible and reliable option. It would allow us to replicate the linting process currently used in GitHub Actions (e.g., using rustfmt directly) without adding extra dependencies. A custom hook can ensure consistent behavior between local checks and CI/CD workflows.

Its exactly what our pre-commit inside the nix-shell does. Since the nix package manager deals with dependencies without maintenance, i dont think this is really a problem... Actually i would follow the complete opposite direction and try to fin more tools that can make our code better, reliable and etc... Not worrying at all about dependencies, we use docker and nix to manage the dependencies of the project and its a matter of good engineering with these tools to increase the "Quality of Life" while developing in Floresta.

I’m leaning toward the second approach for its simplicity and customizability, but I’d love to hear your thoughts before proceeding. If it works for the team, I’m happy to take ownership of this issue and implement the agreed solution and leave your brains to attack more important issues in this project

My thoughts are about documentation and how we deal with things like logs and errors. Removing the dependency on nightly rust to linting would be good too since its too hard to maintain such a thing in a organized way... A good example is what rust-bitcoin did to achieve this. (Spoiler, they use js 👎 )

@JoseSK999
Copy link
Contributor

Can you explain the issue with using nightly for linting? What did rust-bitcoin do?

@lucasbalieiro
Copy link

That would be true only if the pre-commit was doing things differently from our github actions. Which is not the case anymore.

Oh, my mistake! I thought the issue between GitHub Actions and the git hook was still ongoing. I should’ve taken a closer look at the PRs—thanks for pointing that out!

My thoughts are about documentation and how we deal with things like logs and errors. Removing the dependency on nightly rust to linting would be good too since its too hard to maintain such a thing in a organized way...

I might not be able to contribute much to the larger issues just yet, but I can start by reviewing and updating the README to ensure consistency. As I get more familiar with the project, I’ll propose additional ways to help address this issue.

For now, I’m confident I can focus on the README and documentation improvements to make things clearer and more aligned. Let me know if that works!

@Davidson-Souza Davidson-Souza removed this from the v0.6.0 milestone Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Cleaning, refactoring, reducing complexity code quality Generally improves code readability and maintainability dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants