add pin lock in AddallPin function #5506
Conversation
kjzz
force-pushed
the
fix/lock
branch
2 times, most recently
from
a068b19 to
ac7506d
Compare
core/coreunix/add.go
Outdated
| @@ -10,19 +10,19 @@ import ( | |||
| "path/filepath" | |||
| "strconv" | |||
|
|
|||
| core "github.com/ipfs/go-ipfs/core" | |||
| "github.com/ipfs/go-ipfs/core" | |||
There was a problem hiding this comment.
Is there a reason for these changes? They are adding a lot of noise?
There was a problem hiding this comment.
Yeah,actually,it need not to rename the package name.The default name is also core,and renamed maybe redundant.
There was a problem hiding this comment.
There was a (short) discussion on this somewhere, the conclusion IIRC was that for consistency in some places we may actually prefer to have those labels. (some IDEs such as Goland do this by default, you can disable this in Settings > Go > Imports > Optimize imports on the fly)
core/commands/add.go
Outdated
| @@ -276,6 +276,7 @@ You can now check what blocks have been created by: | |||
| fileAdder.NoCopy = nocopy | |||
| fileAdder.Name = pathName | |||
| fileAdder.CidBuilder = prefix | |||
| fileAdder.Hash = hash | |||
There was a problem hiding this comment.
Shouldn't this just set fileAdder.Pin to false?
There was a problem hiding this comment.
why should put fileAdder.Pin to false? The reason i add fileAdder.Hash is that i want to get this param in coreunix/add.go.
There was a problem hiding this comment.
I know, but we don't really need this param there as all it does in disable pinning. See https://github.com/ipfs/go-ipfs/blob/b789295580d2ede0df6e248d73a1a7fb60b253ba/core/coreunix/add.go#L171-L178
We should just do fileAdder.Pin = dopin && hash
There was a problem hiding this comment.
And maybe we should do fileAdder.Pin = dopin && !hash,because hash is mean not to pin file.
|
@magik6k Thx for advice, i have update my pr as you suggested.Please help review again. |
kjzz
force-pushed
the
fix/lock
branch
2 times, most recently
from
d146722 to
47566ec
Compare
|
@magik6k there are some problem about ci.Can u help me restart it and reviwe my pr again?Thx a lot |
|
(Note that you should be able to re-trigger the ci with |
|
Hey @magik6k @Stebalien ,
Because the ci has some error, i did the command |
|
hey @magik6k , if you have anytime. please help me review it again . I have performed a misoperation to dimiss your approved.Very sorry. |
It used to work fine some time ago. Ah well. |
ghost
assigned 
License: MIT Signed-off-by: Kejie Zhang <[email protected]>
License: MIT Signed-off-by: Kejie Zhang <[email protected]>
I enabled that feature but yeah, it's too annoying (now disabled again). I wish there were a less pedantic version (compares the actual diff, not just the commits). |
fix #4561
License: MIT
Signed-off-by: Kejie Zhang [email protected]