Skip to content

lfs,cmd: remove lfs.New{Downloadable,Uploadable}#1776

Merged
ttaylorr merged 3 commits intotq-masterfrom
tq-remove-lfs-funcs
Dec 15, 2016
Merged

lfs,cmd: remove lfs.New{Downloadable,Uploadable}#1776
ttaylorr merged 3 commits intotq-masterfrom
tq-remove-lfs-funcs

Conversation

@ttaylorr
Copy link
Contributor

This pull-request moves the lfs.NewDownloadable and lfs.NewUploadable functions into the commands package and un-exports them.

As an aside, the NewUploadable alternative had to be copied into the test package, since it was inaccessible as a private member of the commands package. I think this follows the Go-ism: "A little copy is better than a little dependency."


/cc @git-lfs/core

@ttaylorr ttaylorr added this to the v2.0.0 milestone Dec 15, 2016
@ttaylorr ttaylorr mentioned this pull request Dec 15, 2016
5 tasks
@ttaylorr
Copy link
Contributor Author

Tracking: #1764.


// ensureFile makes sure that the cleanPath exists before pushing it. If it
// does not exist, it attempts to clean it by reading the file at smudgePath.
func ensureFile(smudgePath, cleanPath string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just get rid of ensureFile() in this test command. It's generating the data on the fly, so I'm not sure how it'd ever get into a weird state.

Copy link
Contributor Author

@ttaylorr ttaylorr Dec 15, 2016

Choose a reason for hiding this comment

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

Great call! Got it in: d8fc641. Once that passes, I'll merge this PR into master.

Copy link
Contributor

@technoweenie technoweenie left a comment

Choose a reason for hiding this comment

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

Looks great 🤘

@ttaylorr ttaylorr merged commit 85242c2 into tq-master Dec 15, 2016
@ttaylorr ttaylorr deleted the tq-remove-lfs-funcs branch December 15, 2016 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants