-
-
Notifications
You must be signed in to change notification settings - Fork 537
Feat/1117 auto content type #1234
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
base: main
Are you sure you want to change the base?
Conversation
06a0615
to
a244618
Compare
I removed the I handled One test fails Ready for review, @epi052 . No rush :-) |
I'm puzzled by how tests behave differently locally and on the CI LocallyWhether I run
Same goes when I run
Env:
CI
no sign of nlp::model::tests::model_generates_expected_tf_idf_scores. This difference of behavior together with the fact that I'm quite new to rust and do not understand in details the architecture of feroxbuster makes tests debugging pretty hard for me. Are those false positives? |
@zar3bski don't sweat those two tests. usually when i see a difference between ci/local tests that pass/fail differently, it's because my local toolchain is behind what runs in CI. If you do a As for the tests themselves, it's not code that's related to your changes. I'll take a look before we merge your branch and get things sorted. Same goes for clippy (which are just new lints in the latest toolchain version). |
ok, i did some thinking about this and looked at how some other cli tools handle this sort of thing (if they do at all). Below is what I'm thinking at this point, please let me know what you think!
|
@epi052 ready for your review. Two comments on it.
|
@epi052 You're probably busy so no rush but I would like arbitrate point 1. and finish this PR in the next following weeks. Tell me what you prefer. |
@zar3bski apologies, i've been preoccupied with life stuff recently. for #1, the enum is fine, go ahead and add comments etc. for #2, you need to add |
Don't worry about the delay, @epi052 , no problem at all (I'm not the quick type). I finally understood why I did not get any logs from
logger::initialize(Arc::new(Configuration::default()))?
let config = Arc::new(Configuration::new().with_context(|| "Could not create Configuration")?);
// setup logging based on the number of -v's used
if matches!(
config.output_level,
OutputLevel::Default | OutputLevel::Quiet
) {
// don't log on --silent
logger::initialize(config.clone())?; // PB: what to do here?
} Calling
Let me know what you think when you have time (life first!). |
Landing a Pull Request (PR)
Long form explanations of most of the items below can be found in the CONTRIBUTING guide.
Branching checklist
Static analysis checks
cargo fmt
clippy
checks pass when runningcargo clippy --all-targets --all-features -- -D warnings -A clippy::mutex-atomic
I do not understand the clipy erros yet, just noticing it while submitting this PR
Documentation
docs
, as needed. The docs live in a separate repository. Update the appropriate pages at the links below.Not yet
Additional Tests
Not