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.

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

Feature/check sum generator #234

Merged
merged 13 commits into from
Jan 27, 2022

Conversation

Joaolfelicio
Copy link
Contributor

@Joaolfelicio Joaolfelicio commented Jan 24, 2022

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Internationalization and localization
  • Other (please describe):

What is the current behavior?

As a user, I'd like to be able to calculate the checksum of a file and compare it with a hash value so I'm able to know if the file was modified.

Issue Number: #141

What is the new behavior?

As a user, I'd like to be able to calculate the checksum of a file using multiple hashing algorithms like MD5, SHA1, SHA256, SHA384 or SHA512.

It will display a loading spinner while a file is being hashed unless the file is small, so we don't see the load flicking.

Other information

main-page

check-sum

check-sum-hashing-algos

check-sum-loading

check-sum-output

Quality check

Before creating this PR, have you:

  • Followed the code style guideline as described in CONTRIBUTING.md
  • Verified that the change work in Release build configuration
  • Checked all unit tests pass

What's missing

  1. Not sure which font icon/svg to use, open to suggestions!
  2. For the moment, the control output comparer is not doing anything, not sure if we want to keep it or what would be the best UX for this control.
  3. I did realize that the compact overlay button on the top sometimes throws, could be local as no change related to it was performed.

I will need some help with the above points, thanks.

@Joaolfelicio
Copy link
Contributor Author

Regarding point number 3, seems like it's only in debug mode!

Copy link
Collaborator

@veler veler left a comment

Choose a reason for hiding this comment

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

Hello,
Thank you very much for this contribution! Overall, it looks very good. Just a few details here and there. :-)

@veler veler added the enhancement Idea of improvement of existing feature. label Jan 25, 2022
@veler veler linked an issue Jan 25, 2022 that may be closed by this pull request
@veler veler added this to the v1.0.2.0 milestone Jan 25, 2022
Copy link
Collaborator

@veler veler left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for the update! It looks good to me. @btiteux , can you please take a look and merge if it looks good to you too?

@Joaolfelicio
Copy link
Contributor Author

Nice! Could you please just take a look at points number 1 and 2?

@veler
Copy link
Collaborator

veler commented Jan 26, 2022

  • Not sure which font icon/svg to use, open to suggestions!
  • For the moment, the control output comparer is not doing anything, not sure if we want to keep it or what would be the best UX for this control.

Oh sorry I missed that.

Not sure which font icon/svg to use, open to suggestions!

It looks good to me. Worst case we can change that later.

For the moment, the control output comparer is not doing anything, not sure if we want to keep it or what would be the best UX for this control.

That's fine too :)

}

_isCalculationInProgress = false;
ShouldDisplayProgress = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm facing a race condition on my machine and I think that this line along with 343 and maybe a few others are the cause.

The scenario I have is:

  1. Select a file. Computation start. Progress bar is displayed.
  2. Select another file while the computation of the 1st one isn't done yet.
  3. In this scenario, the progress bar simply disappear.

I think the race here is that line 343 (with the new file) runs before 356 (with the old file) is executed.

A proper fix is probably to cache the task on line 326 and await it (when not null) right after line 339. Thoughts?

@veler veler removed this from the v1.0.2.0 milestone Jan 27, 2022
@btiteux btiteux merged commit 3c1bac2 into DevToys-app:main Jan 27, 2022
veler pushed a commit that referenced this pull request Mar 31, 2023
* Add CheckSumGenerator boilerplate files

* Add progress loading for the hashing calculation

* Refactor compute hash helper

* Refactor incorrect variable name

* Add HashingHelper unit tests

* Update rews files

* Add compute hash iterations logic

* Short circuit CheckSum if file is null

* Disable hashing dropdown while processing

Apply code review suggestions

* Revert assembly info version

* Apply code review changes

* Remove ControlsToolKit xaml import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Idea of improvement of existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checksum generator
3 participants