Skip to content

commands/fetch: unify formatting#2758

Merged
ttaylorr merged 5 commits intomasterfrom
fetch-unify-formatting
Dec 5, 2017
Merged

commands/fetch: unify formatting#2758
ttaylorr merged 5 commits intomasterfrom
fetch-unify-formatting

Conversation

@ttaylorr
Copy link
Contributor

@ttaylorr ttaylorr commented Nov 29, 2017

This pull request unifies the formatting of git-lfs-fetch(1)'s output, similarly to #2757.

Before:

~/repository (master) $ git lfs fetch --all
Scanning for all objects ever referenced...
Fetching objects...
✔ 501 objects found, done
Git LFS: (291 of 291 files) 1.1 KB / 1.1 KB, done

After:

~/repository (master) $ git lfs fetch --all
fetch: Fetching all references...
fetch: 501 object(s) found, done
Git LFS: (501 of 501 files) 1.9 KB / 1.9 KB, done

This removes the later of the two remaining uses of the *progress.Spinner type, which will allow it to be removed entirely in a subsequent pull request.

Similar to #2757:

Please let me know if you have any thoughts on the formatting of these messages. I tried to make them as similar as I would imagine Git writing them, with these thoughts in mind:

  1. Lowercase prefix of the command that is being run.
  2. ", done\n" on the end (courtesy of *tasklog.Logger).
  3. Use percentages where appropriate, text where otherwise.

/cc @git-lfs/core

@ttaylorr ttaylorr added this to the v2.4.0 milestone Nov 29, 2017
@ttaylorr ttaylorr mentioned this pull request Nov 29, 2017
Merged
technoweenie
technoweenie reviewed Dec 4, 2017
View reviewed changes
commands/command_fetch.go Outdated
// Fetch refs sequentially per arg order; duplicates in later refs will be ignored
for _, ref := range refs {
Print("Fetching %v", ref.Name)
Print("fetch: Fetching reference %s", ref.Name)
Copy link
Contributor

@technoweenie technoweenie Dec 1, 2017

Choose a reason for hiding this comment

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

What about ref.Refspec()? Would you rather see:

fetch: Fetching reference master
fetch: Fetching reference refs/heads/master

Or change reference to the object type:

fetch: Fetching branch master
fetch: Fetching tag v1.0

Copy link
Contributor Author

@ttaylorr ttaylorr Dec 5, 2017

Choose a reason for hiding this comment

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

Interesting question; I think that I prefer the fully-qualified refspec, since downstream parsers can disambiguate overloaded references. (82d68d3).

commands/command_fetch.go
func scanAll() []*lfs.WrappedPointer {
// This could be a long process so use the chan version & report progress
Print("Scanning for all objects ever referenced...")
task := tasklog.NewSimpleTask()
Copy link
Contributor

@technoweenie technoweenie Dec 4, 2017

Choose a reason for hiding this comment

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

I'd add a defer task.Complete() for code consistency.

Copy link
Contributor Author

@ttaylorr ttaylorr Dec 5, 2017

Choose a reason for hiding this comment

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

Good call -- I picked this one up in 0661a8b.

@ttaylorr ttaylorr merged commit b30e65a into master Dec 5, 2017
@ttaylorr ttaylorr deleted the fetch-unify-formatting branch December 5, 2017 22:05
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
@chrisd8088
commands,docs: drop fetch progress prefix
1c0e89a 
In PR git-lfs#2758 the output of the "git lfs fetch" command was revised to
use the methods of the Logger structure of the "tasklog" package to
output progress messages, replacing use of the Spinner structure from
the older "progress" package.  The Spinner structure and its methods
were then removed in PR git-lfs#2759, and the "progress" package was removed
entirely in PR git-lfs#2762.

As part of this change, the "git lfs fetch" command's progress messages
were reformatted so as to always begin with the Git LFS subcommand name,
for example, "fetch: Fetching reference refs/heads/main".

Two other Git LFS commands follow the same design, the "git lfs prune"
and "git lfs migrate" commands, but no others.  Git commands also do
not prefix their progress messages with the name of the subcommand
(although messages from a Git remote may be reported with the prefix
"remote:").

To help bring all our commands' progress messages into alignment, we
simply remove the "fetch:" prefix from the messages output by the
"git lfs fetch" command.  We have similarly altered the progress messages
of the "git lfs prune" command in a previous commit in this PR, and we
expect to revise the progress messages from the "git lfs migrate"
command in the same way in a subsequent commit in this PR.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
@chrisd8088
commands,docs: drop fetch progress prefix
8488488 
In PR git-lfs#2758 the output of the "git lfs fetch" command was revised to
use the methods of the Logger structure of the "tasklog" package to
output progress messages, replacing use of the Spinner structure from
the older "progress" package.  The Spinner structure and its methods
were then removed in PR git-lfs#2759, and the "progress" package was removed
entirely in PR git-lfs#2762.

As part of this change, the "git lfs fetch" command's progress messages
were reformatted so as to always begin with the Git LFS subcommand name,
for example, "fetch: Fetching reference refs/heads/main".

Two other Git LFS commands follow the same design, the "git lfs prune"
and "git lfs migrate" commands, but no others.  Git commands also do
not prefix their progress messages with the name of the subcommand
(although messages from a Git remote may be reported with the prefix
"remote:").

To help bring all our commands' progress messages into alignment, we
simply remove the "fetch:" prefix from the messages output by the
"git lfs fetch" command.  We have similarly altered the progress messages
of the "git lfs prune" command in a previous commit in this PR, and we
expect to revise the progress messages from the "git lfs migrate"
command in the same way in a subsequent commit in this PR.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 25, 2025
@chrisd8088
commands,docs: drop fetch progress prefix
4fde17e 
In PR git-lfs#2758 the output of the "git lfs fetch" command was revised to
use the methods of the Logger structure of the "tasklog" package to
output progress messages, replacing use of the Spinner structure from
the older "progress" package.  The Spinner structure and its methods
were then removed in PR git-lfs#2759, and the "progress" package was removed
entirely in PR git-lfs#2762.

As part of this change, the "git lfs fetch" command's progress messages
were reformatted so as to always begin with the Git LFS subcommand name,
for example, "fetch: Fetching reference refs/heads/main".

Two other Git LFS commands follow the same design, the "git lfs prune"
and "git lfs migrate" commands, but no others.  Git commands also do
not prefix their progress messages with the name of the subcommand
(although messages from a Git remote may be reported with the prefix
"remote:").

To help bring all our commands' progress messages into alignment, we
simply remove the "fetch:" prefix from the messages output by the
"git lfs fetch" command.  We have similarly altered the progress messages
of the "git lfs prune" command in a previous commit in this PR, and we
expect to revise the progress messages from the "git lfs migrate"
command in the same way in a subsequent commit in this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@technoweenie technoweenie technoweenie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

review

Projects

None yet

Milestone

v2.4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ttaylorr @technoweenie