Skip to content

Comprehensive repository improvements: fix type bug, reduce package size, and improve maintainability#4

Merged
jhg merged 5 commits intomainfrom
copilot/fix-ac456838-fc2f-4f60-82f5-707be91cb128
Jul 9, 2025
Merged

Comprehensive repository improvements: fix type bug, reduce package size, and improve maintainability#4
jhg merged 5 commits intomainfrom
copilot/fix-ac456838-fc2f-4f60-82f5-707be91cb128

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 9, 2025

This PR addresses the requested repository review with improvements prioritized as follows:

🔧 Critical Bug Fix

  • Fixed type error in get_user_id_of(): Changed incorrect return type from libc::gid_t to libc::uid_t. This was a potentially serious bug that could cause incorrect behavior when setting user IDs.

📦 Package Size Reduction

  • Made log dependency optional: The log crate is now an optional dependency behind the logging feature (enabled by default)
  • Users can opt out with: drop-root = { version = "1.0", default-features = false }
  • This reduces the dependency footprint for users who don't need logging

🛠️ Code Quality & Maintainability

  • Fixed all clippy warnings: Updated format strings to use modern inline syntax ({variable} instead of {}, variable)
  • Improved error messages: Fixed inconsistent error message for setgroups operation
  • Removed unnecessary empty line after outer attribute in lib.rs

🔄 CI/CD Improvements

  • Updated GitHub Actions: Replaced deprecated actions/checkout@v2 and actions-rs/toolchain@v1 with modern alternatives
  • Added feature testing: CI now validates the crate works both with and without the logging feature
  • Enhanced linting: Added clippy checks to CI pipeline

📚 Documentation

  • Updated README: Added documentation for the new optional logging feature
  • Fixed doctest: Removed unnecessary fn main() wrapper in README example

Testing

All existing tests pass, and the changes are backward compatible. The crate now:

  • ✅ Compiles with default features (logging enabled)
  • ✅ Compiles with --no-default-features (logging disabled)
  • ✅ Passes all clippy lints
  • ✅ Maintains the same public API

The changes are minimal and surgical, focusing on fixing the critical bug while improving code quality and reducing optional dependencies.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI assigned Copilot and jhg Jul 9, 2025
@jhg jhg changed the title [WIP] haz una revisión de todo el repositorio para: - revisar dependencias que deban actualizarse - simplificar código para que sea más fácil de mantener y más seguro al ser más fácil revisarlo - mejoras de usabilidad para que sea cómodo de usarse - r... [WIP] simplify codebase and keep updated Jul 9, 2025
…ge size, and improve maintainability

Co-authored-by: jhg <1288711+jhg@users.noreply.github.com>
Copilot AI changed the title [WIP] simplify codebase and keep updated Comprehensive repository improvements: fix critical type bug, reduce package size, and improve maintainability Jul 9, 2025
Copilot AI requested a review from jhg July 9, 2025 22:13
Copilot finished work on behalf of jhg July 9, 2025 22:13
@jhg jhg changed the title Comprehensive repository improvements: fix critical type bug, reduce package size, and improve maintainability Comprehensive repository improvements: fix type bug, reduce package size, and improve maintainability Jul 9, 2025
@jhg jhg marked this pull request as ready for review July 9, 2025 22:15
Copy link
Copy Markdown
Owner

@jhg jhg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can reduce uploaded size of package if in Cargo.toml you use something like:

exclude = [
    ".github/**",
    ".vscode/**",
    ".idea/**",
    "tests/**", # To reduce the package size, as tests are not needed to use it.
]

or:

exclude = [".github/**", ".vscode/**", "benches/**", "Cargo.lock", "LICENSE", ".gitignore"]

Comment thread .github/workflows/ci.yml
Comment on lines 35 to +42
- name: Run tests
run: cargo test -- --show-output

- name: Check without logging feature
run: cargo check --no-default-features

- name: Run clippy
run: cargo clippy --all-targets -- -D warnings
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok, but could be nice also run tests without logging feature.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! I've added a dedicated step to run tests without the logging feature. The CI now includes both cargo check --no-default-features and cargo test --no-default-features to ensure the crate works properly in both configurations. (3571bbf)

Co-authored-by: jhg <1288711+jhg@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jul 9, 2025

You can reduce uploaded size of package if in Cargo.toml you use something like:

...

Good suggestion! I've expanded the exclude list in Cargo.toml to include .vscode/**, .idea/**, tests/**, benches/**, and Cargo.lock to further reduce the package upload size. This will help keep the published crate smaller by excluding development files. (3571bbf)

Copilot AI requested a review from jhg July 9, 2025 22:40
Copilot finished work on behalf of jhg July 9, 2025 22:40
@jhg jhg merged commit ae9eeb6 into main Jul 9, 2025
1 check passed
@jhg jhg deleted the copilot/fix-ac456838-fc2f-4f60-82f5-707be91cb128 branch July 9, 2025 22:49
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 this pull request may close these issues.

2 participants