[build] Refactor Repository Structure#36
Conversation
Summary of ChangesHello @csegarragonz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the project's repository structure and build system. The main CLI tool has been renamed from 'invrs' to 'accli', and the entire project has transitioned to a Cargo workspace for streamlined dependency management and consistent versioning across all Rust crates. Configuration files have been centralized, and new developer tools for automated versioning and code formatting have been integrated, enhancing overall project organization and developer experience. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and beneficial refactoring of the repository structure into a Rust workspace. This centralizes dependency management and improves project organization. However, there are several critical issues with the new build and dependency setup that need to be addressed.
Most importantly, the workspace is configured to use the Rust 2024 edition, which is not yet stable and will cause build failures with the specified stable toolchain. Additionally, the build script for workflow drivers uses incorrect package names, which will also break the build.
There are also significant issues with dependency resolution, resulting in multiple versions of key crates like base64, http, hyper, nom, and reqwest being included in the build. This can lead to increased binary size, longer compile times, and potential runtime errors. I recommend running cargo tree -d to analyze the dependency graph and unify versions where possible. Specific suggestions for some of these are included in the comments.
accless/libs/jwt/src/lib.rs
Outdated
| if let Some(obj) = header.as_object() && obj.contains_key(property) { | ||
| let value = obj | ||
| .get(property) | ||
| .and_then(|value| value.as_str().map(|s| s.to_string())) | ||
| .unwrap(); | ||
| return value == exp_value; | ||
| } | ||
|
|
||
| // Check in body | ||
| if let Some(obj) = payload.as_object() { | ||
| if obj.contains_key(property) { | ||
| let value = obj | ||
| .get(property) | ||
| .and_then(|value| value.as_str().map(|s| s.to_string())) | ||
| .unwrap(); | ||
| return value == exp_value; | ||
| } | ||
| if let Some(obj) = payload.as_object() && obj.contains_key(property) { | ||
| let value = obj | ||
| .get(property) | ||
| .and_then(|value| value.as_str().map(|s| s.to_string())) | ||
| .unwrap(); | ||
| return value == exp_value; | ||
| } |
There was a problem hiding this comment.
The use of .unwrap() here is unsafe and can cause a panic if the JWT property exists but is not a string. This function should handle malformed JWTs gracefully instead of panicking. You can use if let or matches! for a safer check.
| if let Some(obj) = header.as_object() && obj.contains_key(property) { | |
| let value = obj | |
| .get(property) | |
| .and_then(|value| value.as_str().map(|s| s.to_string())) | |
| .unwrap(); | |
| return value == exp_value; | |
| } | |
| // Check in body | |
| if let Some(obj) = payload.as_object() { | |
| if obj.contains_key(property) { | |
| let value = obj | |
| .get(property) | |
| .and_then(|value| value.as_str().map(|s| s.to_string())) | |
| .unwrap(); | |
| return value == exp_value; | |
| } | |
| if let Some(obj) = payload.as_object() && obj.contains_key(property) { | |
| let value = obj | |
| .get(property) | |
| .and_then(|value| value.as_str().map(|s| s.to_string())) | |
| .unwrap(); | |
| return value == exp_value; | |
| } | |
| if header.get(property).and_then(|v| v.as_str()) == Some(exp_value) { | |
| return true; | |
| } | |
| // Check in body | |
| if payload.get(property).and_then(|v| v.as_str()) == Some(exp_value) { | |
| return true; | |
| } |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
8e71032 to
396847d
Compare
No description provided.