Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: auto detect some common filenames of hash file #146

Merged
merged 1 commit into from
Nov 27, 2024
Merged

feat: auto detect some common filenames of hash file #146

merged 1 commit into from
Nov 27, 2024

Conversation

Integral-Tech
Copy link
Contributor

Some Linux distributions use specific name for hash files, including but not limited to:

Therefore, we can detect filenames of these hash files automatically in the directory of image file.

@Integral-Tech
Copy link
Contributor Author

@ifd3f

ifd3f
ifd3f requested changes Nov 15, 2024
View reviewed changes
Copy link
Owner

@ifd3f ifd3f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for taking so long to get to you, I have been somewhat burnt out and I'm slowly getting back into things. I have put in several code style suggestions -- would you like to try to do them yourself, or would you like me to make those changes?

src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@Integral-Tech Integral-Tech requested a review from ifd3f November 16, 2024 08:22
@Integral-Tech
Copy link
Contributor Author

@ifd3f std::fs::exists requires Rust 1.81+, therefore the Rust version in CI should be updated

@ifd3f
Copy link
Owner

ifd3f commented Nov 18, 2024

Yes, I'm currently working on that.

ifd3f
ifd3f requested changes Nov 18, 2024
View reviewed changes
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
@ifd3f ifd3f self-assigned this Nov 18, 2024
@Integral-Tech Integral-Tech force-pushed the auto-detect-hashfile branch 4 times, most recently from 391a9ba to 778d7a0 Compare November 18, 2024 11:26
@Integral-Tech Integral-Tech requested a review from ifd3f November 18, 2024 13:32
@ifd3f
Copy link
Owner

ifd3f commented Nov 19, 2024

For the version update, I've created #151 to track it. I'll try to push out a PR that fixes that Soon(tm)

@Integral-Tech
Copy link
Contributor Author

Integral-Tech commented Nov 19, 2024 •
edited
Loading

@ifd3f Could you please approve my PR now?

@ifd3f
Copy link
Owner

ifd3f commented Nov 19, 2024

@ifd3f Could you please approve my PR now?

Not yet, there's still a couple of code style changes I would like before it can be merged.

@Integral-Tech
Copy link
Contributor Author

@ifd3f Could you please approve my PR now?

Not yet, there's still a couple of code style changes I would like before it can be merged.

You can just request changes and I will fix them

@Integral-Tech Integral-Tech force-pushed the auto-detect-hashfile branch 4 times, most recently from a4137da to 610d5c2 Compare November 25, 2024 07:53
ifd3f
ifd3f requested changes Nov 27, 2024
View reviewed changes
Copy link
Owner

@ifd3f ifd3f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last few changes that i think you should make -- sorry it took a bit to get back to you, but it should be good after these last two things. thanks for the PR!

src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
src/ui/simple_ui/ask_hash.rs Outdated Show resolved Hide resolved
@Integral-Tech
Copy link
Contributor Author

@ifd3f Done :)

@ifd3f ifd3f enabled auto-merge November 27, 2024 08:17
@Integral-Tech
Copy link
Contributor Author

@ifd3f Should I squash them into a single commit before getting merged?

@ifd3f
Copy link
Owner

ifd3f commented Nov 27, 2024

@ifd3f Should I squash them into a single commit before getting merged?

Yeah, probably a good idea

@ifd3f ifd3f disabled auto-merge November 27, 2024 09:40
@Integral-Tech
Copy link
Contributor Author

@ifd3f Done :)

@ifd3f ifd3f merged commit be0b9bf into ifd3f:main Nov 27, 2024
24 checks passed
@Integral-Tech Integral-Tech deleted the auto-detect-hashfile branch November 28, 2024 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ifd3f ifd3f ifd3f approved these changes

Assignees

@ifd3f ifd3f

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Integral-Tech @ifd3f