Skip to content

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Dec 6, 2022

PR Summary

Copy-Item for FileSystemProvider already has progress for copying to/from PSSession, but nothing for local copies. So for a large copy, the cmdlet just sits until it completes (although -Verbose could be used, but it's noisy and not the right way to get progress). Change here is to have two progress bars, one when copying a file and another when copying a directory. For directories, we can get the number of files within the directory and show percent complete. However, we cannot get the total files/folders when used recursively (without incurring a huge up front cost to enumerate it all) so the progress doesn't show the total operation percent complete, but does indicate what is happening.

Screen-Recording-2022-12-05-at-4

PR Context

Fix #18637

PR Checklist

@pull-request-quantifier-deprecated

This PR has 42 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

Label      : Extra Small
Size       : +42 -0
Percentile : 16.8%

Total files changed: 2

Change summary by file extension:
.cs : +33 -0
.resx : +9 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

  • Fast and predictable releases to production:
    • Optimal size changes are more likely to be reviewed faster with fewer
      iterations.
    • Similarity in low PR complexity drives similar review times.
  • Review quality is likely higher as complexity is lower:
    • Bugs are more likely to be detected.
    • Code inconsistencies are more likely to be detected.
  • Knowledge sharing is improved within the participants:
    • Small portions can be assimilated better.
  • Better engineering practices are exercised:
    • Solving big problems by dividing them in well contained, smaller problems.
    • Exercising separation of concerns within the code changes.

What can I do to optimize my changes

  • Use the PullRequestQuantifier to quantify your PR accurately
    • Create a context profile for your repo using the context generator
    • Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
    • Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
    • Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
  • Change your engineering behaviors
    • For PRs that fall outside of the desired spectrum, review the details and check if:
      • Your PR could be split in smaller, self-contained PRs instead
      • Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

  • One line was added: +1 -0
  • One line was deleted: +0 -1
  • One line was modified: +1 -1 (git diff doesn't know about modified, it will
    interpret that line like one addition plus one deletion)
  • Change percentiles: Change characteristics (addition, deletion, modification)
    of this PR in relation to all other PRs within the repository.


Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

@ghost ghost assigned iSazonov Dec 6, 2022
@SteveL-MSFT SteveL-MSFT added the WG-Interactive-Console the console experience label Dec 6, 2022
@kilasuit
Copy link
Collaborator

kilasuit commented Dec 6, 2022

for me there is far to much movement in the UI for this to be of any use
It needs some static elements to be of real use as things stand from the clip shared

@MartinGC94
Copy link
Contributor

Nice. I was a little worried that this would impact the performance, but I tested it with a lot of small files (2.4GB, spread across 8613 files) and the performance stayed the same (around 8.8 seconds). Robocopy is still faster at around 6.1 seconds but that's hard to compete with.

@SteveL-MSFT
Copy link
Member Author

The initial copy found a bunch of hash files in the "hidden" .git folder, so that's the hex numbers you see at the start. However, under normal circumstances if copying lots of folders or larger files, then the content won't jump around so much.

@iSazonov
Copy link
Collaborator

iSazonov commented Dec 6, 2022

It seems to me that such things are not useful in the sense that they do not solve the problems that they should.
If you have to constantly copy files, then after the first admiration comes the annoyance of useless flickering on the screen.
Does anyone really have time to read and remember file names in this progress bar? Really helps you? Really?

The main thing the progress bar is for is the frozen screen. When a user gets worried that the process is frozen? Usually it is 5-15 seconds. This means that there is no point in displaying the progress bar before this time has elapsed.

The second purpose of the progress bar is to show useful information for the user.
Here we have two things (related to the frozen screen problem).
First, it makes no sense to show file names if you copy many small files. It is enough to show number of copied files (and probably total time/size of copying).
Second, there are situations of long copying of a large file. In this case it makes sense to display the file name and the copying progress to avoid the frozen screen problem - the last does not exist now.

I would like to have an API that allows to do such things OOB and save (module) developers (see for ex., ADUC or EMS (Exchange Server)) from duplication.

@SteveL-MSFT SteveL-MSFT changed the title Add progress to FileSystemProvider for Copy-Item WIP: Add progress to FileSystemProvider for Copy-Item Dec 6, 2022
@SteveL-MSFT
Copy link
Member Author

Let me try a different approach as a separate PR to see if the perf isn't significantly impacted

@SteveL-MSFT SteveL-MSFT mentioned this pull request Dec 6, 2022
22 tasks
@SteveL-MSFT
Copy link
Member Author

See if #18735 is preferred although there is a bigger perf penalty

@SteveL-MSFT
Copy link
Member Author

Closing this in favor of the other one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copy-Item add support "report progress"

4 participants