refactor: code cleanup #2
Conversation
DavSanchez
force-pushed
the
refactor/review-code
branch
from
701edf8 to
e2f0c76
Compare
DavSanchez
force-pushed
the
refactor/code-cleanup
branch
2 times, most recently
from
b2200b3 to
3359d63
Compare
DavSanchez
force-pushed
the
refactor/review-code
branch
2 times, most recently
from
95caf40 to
d2865a6
Compare
DavSanchez
force-pushed
the
refactor/code-cleanup
branch
3 times, most recently
from
a0f0083 to
0843d92
Compare
| let key_split = key.split_whitespace().collect::<Vec<_>>(); | ||
|
|
||
| match key_split[..] { | ||
| [crate_name, _crate_version, url] => Ok(NameAndUrl(format!( |
There was a problem hiding this comment.
This drops the crate version from the input so it's not usable elsewhere in the code. Reduces complexity for the templates, but performs the changes via code logic so it's less flexible. Let me know what you prefer.
| fn process_name_with_url(dep_name: &str, url_str: &str) -> String { | ||
| match url_str { | ||
| // crates.io | ||
| "registry+https://github.com/rust-lang/crates.io-index" => { | ||
| format!("https://crates.io/crates/{}", dep_name) | ||
| } | ||
| // remove any prefix for other registries, up until the + symbol | ||
| s if s.starts_with("git+") => remove_source_prefix(s).trim_end_matches(".git").to_string(), | ||
| s => remove_source_prefix(s), | ||
| } | ||
| } |
There was a problem hiding this comment.
This was originally done in the template itself though only for crates.io references (not git repos). It reduces complexity for the templates (as I find them harder to test), but performs the changes via code logic so it's less flexible for other use cases. Let me know what you prefer.
| fn remove_source_prefix(url_str: &str) -> String { | ||
| // Is there a registry indicator? We know it's a registry if it has a + symbol at the end. | ||
| let sum_symbol_index = url_str.find('+').map(|i| i + 1).unwrap_or(0); | ||
| url_str.chars().skip(sum_symbol_index).collect::<String>() | ||
| } |
There was a problem hiding this comment.
This was originally done in the template itself though only for crates.io references (not git repos). It reduces complexity for the templates (as I find them harder to test), but performs the changes via code logic so it's less flexible for other use cases. Let me know what you prefer.
| - uses: newrelic/rust-licenses-noticer@refactor/code-cleanup | ||
| with: | ||
| template-file: "./THIRD_PARTY_NOTICES.md.tmpl" | ||
| rust-licenses-noticer-branch: "refactor/code-cleanup" |
There was a problem hiding this comment.
Nice!
Using the refactor/code-cleanup branch is a workaround to make it work from here now, right? We'll need to update it.
There was a problem hiding this comment.
Yupp, as soon as it's merged to main I'll fix all references
- Remove `lazy_static` dependency. - Use `PathBuf`s instead of strings where appropriate in CLI args parse. - Move template outside `src` directory. - Reuse code in golden test.
DavSanchez
force-pushed
the
refactor/code-cleanup
branch
from
4e328bf to
fc010e8
Compare
PR performs several things:
serde_json::Valueallowing us to pre-process the data and control the values sent to the template.THIRD_PARTY_NOTICES.mdnow performs less operations (some were delegated to the code) and results on cleaner markdown output.gitis more robust.