mario-eth / soldeer

Solidity Package Manager written in rust
MIT License
143 stars 13 forks source link

✨ Added warning when pushing sensitive files #73

Closed Mario-SO closed 2 weeks ago

Mario-SO commented 2 weeks ago

As discussed in here for issue #67, added a user warning when pushing .env or .git/

PS: Missing tests, looking forward to feedback.

mario-eth commented 2 weeks ago

It seems that we differ in fmt settings? cause i see there are many changes on how the imports are done? i use the foundry ones: https://github.com/foundry-rs/foundry/blob/master/CONTRIBUTING.md

Mario-SO commented 2 weeks ago

It seems that we differ in fmt settings? cause i see there are many changes on how the imports are done?

i use the foundry ones:

https://github.com/foundry-rs/foundry/blob/master/CONTRIBUTING.md

Crap, you are totally right, my bad. Will fix and PR again.

PS: Also saw the rest of the comments you made.

mario-eth commented 2 weeks ago

you have to run cargo +nightly clippy --workspace --all-targets --all-features And resolve them

and cargo +nightly fmt --all --check

Cause the CI fails

Mario-SO commented 2 weeks ago

Ok I see, sorry for all the inconveniences this PR is causing first of all.

Also wanted to ask, when running cargo +nightly clippy --workspace --all-targets --all-features, there is a warning in file dependency_downloader.rs which is a file I didn't touch at all. The warning is this one

warning: this function depends on never type fallback being `()`
  --> src/dependency_downloader.rs:18:1
   |
18 | / pub async fn download_dependencies(
19 | |     dependencies: &[Dependency],
20 | |     clean: bool,
21 | | ) -> Result<(), DownloadError> {
   | |______________________________^
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
   = help: specify the types explicitly
note: in edition 2024, the requirement `!: std::iter::FromIterator<()>` will fail
  --> src/dependency_downloader.rs:35:16
   |
35 |     .collect::<Result<_, _>>()?;
   |                ^^^^^^^^^^^^
   = note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

The rest of the warnings are resolved and also ran the cargo +nightly fmt --all --check.

I will push again and await your feedback once more.

Again sorry to bother this much with such a minor PR.

mario-eth commented 2 weeks ago

Ok I see, sorry for all the inconveniences this PR is causing first of all.

Also wanted to ask, when running cargo +nightly clippy --workspace --all-targets --all-features, there is a warning in file dependency_downloader.rs which is a file I didn't touch at all. The warning is this one

warning: this function depends on never type fallback being `()`
  --> src/dependency_downloader.rs:18:1
   |
18 | / pub async fn download_dependencies(
19 | |     dependencies: &[Dependency],
20 | |     clean: bool,
21 | | ) -> Result<(), DownloadError> {
   | |______________________________^
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
   = help: specify the types explicitly
note: in edition 2024, the requirement `!: std::iter::FromIterator<()>` will fail
  --> src/dependency_downloader.rs:35:16
   |
35 |     .collect::<Result<_, _>>()?;
   |                ^^^^^^^^^^^^
   = note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

The rest of the warnings are resolved and also ran the cargo +nightly fmt --all --check.

I will push again and await your feedback once more.

Again sorry to bother this much with such a minor PR.

ah no worries at all, thank you for taking the time to get this done. That is weird, it seems it's like a new compiler warning, can you solve it?

Mario-SO commented 2 weeks ago

Sure, will solve it asap

mario-eth commented 2 weeks ago

actually i m gonna solve them cause it seems there are more now after I did an update. will solve then if everything is ok I m gonna merge this one

Mario-SO commented 2 weeks ago

It no longer pops any warnings.