search

Ulyssedev / Rust-undetected-chromedriver

A rust implementation of ultrafunkamsterdam's undetected-chromedriver library based on thirtyfour
https://crates.io/crates/undetected-chromedriver
MIT License
39 stars 22 forks source link

refactor #6

Open chungwong opened 11 months ago

chungwong commented 11 months ago

This is a code refactor(with personal taste) PR.

  1. fix https://github.com/Ulyssedev/Rust-undetected-chromedriver/issues/5
  2. add mac aarch64
  3. remove all unwraps
  4. cargo fmt and cargo clippy
  5. favour tracing over println!
Ulyssedev commented 11 months ago

Hey, thanks a lot for contributing, the changes are looking great!

I have a concern regarding the renaming of the chrome function to chrome_driver, the Chrome trait to ChromeDriver, and the new function to web_driver. While consistency is important, these changes might impact existing codebases that use this crate. Could you please provide more context on the reasoning behind these naming changes?

Other than that, the refactor looks promising and will be merged soon, thanks again!

chungwong commented 11 months ago

renaming of the chrome function to chrome_driver

fn chrome is actually returning a WebDriver and it is not concise of what it is about.

the Chrome trait to ChromeDriver

Similar to above, the trait is actually about chrome driver, not Chrome.

and the new function to web_driver

For some reason I cannot reproduce the problem on my laptop which I had this morning on a different computer. But I can illustrate the problem here

But in generally, I believe chrome, chromedriver and webdriver are really confusing. This library mainly focus on chrome driver only and of course thrityfour::WebDriver is more than just chrome driver. I do have the tendency to make it consistent, maybe just use chromdriver at all or stick to webdirver but I have no preference at this moemnt.

I admit I didn't have backward compatability in my mind for too long but I thought we were still in the early stage and the Chrome trait was just less than 24 hours old.

Please feel free to request changes.