Closed FrancoRav closed 3 months ago
Looks great! Just some minor things.
cargo fmt --check
seems to be failing, as well ascargo deny check
. (these are found in theci.sh
script).
BSL-1.0
could be added to the allowed list of licenses in deny.tomlPerhaps the added changelog (
git cliff --unreleased
) could be a bit more descriptive:### ๐ Features - Add copy commit hash feature
Something like: Copy commit hash with "y", move Show refs to "Y" ?
..just to convey that the binding was moved as part of the changelog!
I had never used git cliff before. Is there any way to change the changelog message without force pushing to change the commit message?
The way I've set it up is to re-generate the entire changelog. Perhaps there's better ways to set it up to allow manual editing of commits. But as of now I'm not sure. I haven't used it all that much.
Ah the second commit: fix: update tests to new key bindings
would be visible in the log as well. You could use test:
, or just remove the prefix altogether.
As seen in cliff.toml
commit_parsers = [
{ message = "^feat", group = "<!-- 0 -->๐ Features" },
{ message = "^fix", group = "<!-- 1 -->๐ Bug Fixes" },
# { message = "^doc", group = "<!-- 3 -->๐ Documentation" },
{ message = "^perf", group = "<!-- 4 -->โก Performance" },
# { message = "^refactor", group = "<!-- 2 -->๐ Refactor" },
{ message = "^style", group = "<!-- 5 -->๐จ Styling" },
# { message = "^test", group = "<!-- 6 -->๐งช Testing" },
# { message = "^chore\\(release\\): prepare for", skip = true },
# { message = "^chore\\(deps.*\\)", skip = true },
# { message = "^chore\\(pr\\)", skip = true },
# { message = "^chore\\(pull\\)", skip = true },
# { message = "^chore|^ci", group = "<!-- 7 -->โ๏ธ Miscellaneous Tasks" },
{ body = ".*security", group = "<!-- 8 -->๐ก๏ธ Security" },
{ message = "^revert", group = "<!-- 9 -->โ๏ธ Revert" },
]
I force pushed to reword the commits for the Changelog.
Also, I just noticed the Clipboard::new()
function fails in the test environment as it can't access any clipboard (the error is Unknown error while interacting with the clipboard: $DISPLAY variable not set and no value was provided explicitly
). Not sure how to solve this
I was thinking making the Clipboard an Option<Clipboard>
and leaving None if it errors, but then the feature could not be tested.
Using Option for that is probably necessary anyway, since we need error handling in case it happens in real use, but it still doesn't fix the problem of how to test the feature. Any ideas?
There's not a lot of code, and if we're to test anything, I guess it could be the message? Seems fine wrapping it in an Option!
There's not a lot of code, and if we're to test anything, I guess it could be the message? Seems fine wrapping it in an Option!
That's done now. It should pass ci now, and it should be working correctly. I'm still not sure about the way I am displaying the info and erros in the CmdLog (the methods I added to the State), so if you suggest any changes I can look into that
Attention: Patch coverage is 37.20930%
with 27 lines
in your changes are missing coverage. Please review.
Project coverage is 86.96%. Comparing base (
995f877
) to head (53e0450
).
Files | Patch % | Lines |
---|---|---|
src/ops/copy_hash.rs | 15.78% | 16 Missing :warning: |
src/state.rs | 45.45% | 6 Missing :warning: |
src/cmd_log.rs | 0.00% | 5 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Add "copy commit hash" feature, bound to "y" by default. Changes default keybinding for show refs to "Y", as suggested in #119 Closes #119