Code structure #425
Code structure #425
Conversation
…ource_assembler.rs
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Rust Benchmark
| Benchmark suite | Current: d5a6a67 | Previous: 7919bdd | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
1766321560 ns/iter (± 17538388) |
1745226241 ns/iter (± 10688991) |
1.01 |
rule-match-first-request/brave-list |
1018953 ns/iter (± 7037) |
1003256 ns/iter (± 7610) |
1.02 |
blocker_new/brave-list |
209918857 ns/iter (± 4443194) |
210108247 ns/iter (± 7007989) |
1.00 |
memory-usage/brave-list-initial |
41409969 ns/iter (± 3) |
41409969 ns/iter (± 3) |
1 |
memory-usage/brave-list-after-1000-requests |
44005995 ns/iter (± 3) |
44005995 ns/iter (± 3) |
1 |
This comment was automatically generated by workflow using github-action-benchmark.
boocmp
force-pushed
the
code_structure
branch
from
6c8853b to
8e315c2
Compare
boocmp
force-pushed
the
code_structure
branch
from
8e315c2 to
48df927
Compare
| @@ -894,13 +941,17 @@ impl NetworkFilter { | |||
| /// emulate the behavior of hosts-style blocking. | |||
| pub fn parse_hosts_style(hostname: &str, debug: bool) -> Result<Self, NetworkFilterError> { | |||
| // Make sure the hostname doesn't contain any invalid characters | |||
| static INVALID_CHARS: Lazy<Regex> = Lazy::new(|| Regex::new("[/^*!?$&(){}\\[\\]+=~`\\s|@,'\"><:;]").unwrap()); | |||
| static INVALID_CHARS: Lazy<Regex> = | |||
| Lazy::new(|| Regex::new("[/^*!?$&(){}\\[\\]+=~`\\s|@,'\"><:;]").unwrap()); | |||
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] expect or unwrap called in function returning a Result
Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result
Cc @thypon @kdenhartog
| let original_rule = *filter | ||
| .raw_line | ||
| .clone() | ||
| .expect("All rules should be in debug mode"); |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] expect or unwrap called in function returning a Result
Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result
Cc @thypon @kdenhartog
| let original_rule = *filter | ||
| .raw_line | ||
| .clone() | ||
| .expect("All rules should be in debug mode"); |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] expect or unwrap called in function returning a Result
Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result
Cc @thypon @kdenhartog
|
|
||
| /* | ||
| #[cfg(test)] | ||
| mod optimization_tests_union_domain { |
There was a problem hiding this comment.
a duplicate? about the same block on line 190.
There was a problem hiding this comment.
In this PR I just moved the code and fixed the compilation. The code moved as is, but I guess I can clean up it as well if everyone don't mind.
|
@boocmp what exactly is the justification for moving all the tests? Codebase churn and personal opinions aside, it's considered idiomatic Rust to keep the unit tests close to the corresponding implementations. Ref:
|
| @@ -1,77 +0,0 @@ | |||
| use regex::Regex; | |||
|
|
|||
| pub fn get_hostname_regex(url: &str) -> Option<(usize, (usize, usize))> { | |||
The first reason is to reduce the amount of code in files. It’s difficult to work with a file that has over a thousand lines of code, especially when most of it consists of tests. For example, a task like adding code to the end of a file becomes quite tedious you have to search for where the tests begin instead of simply pressing the End key. The second reason is search. I can configure a search for any substring while ignoring tests by simply excluding the test directory. The third reason is GitHub diffs. It’s much clearer to see where changes are made in the code versus in the tests, since GitHub only shows a few lines around the changes, it’s not always obvious otherwise. I’m not an expert in Rust, but this recommendation goes against everything I’ve seen in other languages and projects throughout my experience. |
The test code has been moved to the test/unit/ folder, replicating the src directory structure. In my opinion, this improves the efficiency of working with the codebase.