8000 Add basic Lock implementation by sdboyer · Pull Request #5 · golang/dep · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Add basic Lock implementation - #5

Merged
sdboyer merged 2 commits into
golang:masterfrom
sdboyer:basic-lock
Oct 18, 2016
Merged

Add basic Lock implementation#5
sdboyer merged 2 commits into
golang:masterfrom
sdboyer:basic-lock

Conversation

@sdboyer
@sdboyer sdboyer commented Oct 18, 2016
Copy link
Copy Markdown
Member

Patterned quite strongly after #4

adg
adg approved these changes Oct 18, 2016
View reviewed changes
@adg adg left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suppose this is okay, for now. I worry that we're baking in some assumptions that gps makes, like the input hash for lock files, but let's see where it takes us.

Comment thread lock.go Outdated
P: make([]gps.LockedProject, len(rl.P)),
}

for k, ld := range rl.P {
@adg adg Oct 18, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'd go with i instead of k, as it's an index not a key

@sdboyer sdboyer Oct 18, 2016
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

mm yes, fair

@sdboyer
sdboyer commented Oct 18, 2016
Copy link
Copy Markdown
Member Author

We could drop that memo prop now (though we'd still need the InputHash() method to fulfill the gps.Lock interface, ofc), but my choice to include it initially here was deliberate:

  • It is quite decoupled from pretty much all the other data, so there's little complexity cost
  • The Solutions returned from gps.Solve() will always have that hash digest available, so it's not like there's much extra work being done
  • Some of our immediately-upcoming choices would be influenced by the use of that hashing system, so it struck me as better to have it in for easy, immediate experimentation purposes than to talk about it in the abstract
  • The patterns of use it facilitates are likely to be helpful in thinking about overall system constraints, even if that's just in deciding we don't want to use it

Are there other assumptions there that concern you?

@sdboyer
sdboyer merged commit f569990 into golang:master Oct 18, 2016
sdboyer added a commit that referenced this pull request Oct 18, 2016
@sdboyer
Add basic Lock implementation (#5)
2f506ee
@sdboyer
sdboyer commented Oct 18, 2016
Copy link
Copy Markdown
Member Author

(merged, given the approval, but i'm happy to still discuss)

@adg
adg commented Oct 18, 2016
Copy link
Copy Markdown
Contributor

Yeah, that all makes sense.

Are there other assumptions there that concern you?

Not that I can articulate right now. Let's see where this leads us.

zbintliff added a commit to zbintliff/dep that referenced this pull request Mar 3, 2017
@zbintliff @sdboyer
Add basic Lock implementation (golang#5)
bff05d8
ibrasho pushed a commit to ibrasho-forks/dep that referenced this pull request May 10, 2017
@sdboyer
Add basic Lock implementation (golang#5)
d5ca101
zjs pushed a commit to zjs/dep that referenced this pull request Mar 28, 2018
@akutz
Merge pull request golang#5 from zjs/topic/status-integration
c354896 
 status: process detail template output all-at-once
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@adg adg adg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sdboyer @adg
0