Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Default logic to files cmd #2667

Closed
wants to merge 1 commit into from

Conversation

RichardLitt
Copy link
Member

Part of #2484

License: MIT
Signed-off-by: Richard Littauer [email protected]

@RichardLitt RichardLitt added topic/docs-ipfs Topic docs-ipfs and removed topic/docs-ipfs Topic docs-ipfs labels May 11, 2016
@Kubuxu
Copy link
Member

Kubuxu commented May 13, 2016

@RichardLitt
Copy link
Member Author

That doesn't look like it is my fault.

@@ -417,11 +414,11 @@ Examples:
res.SetError(err, cmds.ErrNormal)
return
}
if found {
Copy link
Member

Choose a reason for hiding this comment

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

You removed this if block without removing found variable in line 412.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@RichardLitt RichardLitt force-pushed the feature/add-default-to-files branch from 97b49b9 to 91049ec Compare May 14, 2016 14:39
@Kubuxu Kubuxu force-pushed the feature/add-default-to-files branch from 91049ec to f3bd304 Compare May 17, 2016 19:07
@Kubuxu
Copy link
Member

Kubuxu commented May 17, 2016

Here are still commands that miss some of the help texts:
http://ci.ipfs.team:8111/viewLog.html?buildId=1927&buildTypeId=GoIpfs_CiTests&tab=buildLog#_focus=2714

@Kubuxu Kubuxu added the need/author-input Needs input from the original author label May 17, 2016
@RichardLitt RichardLitt force-pushed the feature/add-default-to-files branch from f3bd304 to 4f23a42 Compare May 18, 2016 14:37
@RichardLitt
Copy link
Member Author

RichardLitt commented May 18, 2016

I need some help with this; I am confused how you process this information.

I open up Teamcity. I see a very long file. I see lots of yellow // orange text. Sometimes it is OK, but I don't know how I am supposed to know it is OK or not. I go to the bottom, and I see this:

[14:33:08][Step 4/4] make[1]: *** [t0250-files-api.sh] Error 1
[14:33:08][Step 4/4] 1..251
[14:33:08][Step 4/4] make: *** [test_sharness_expensive] Error 2
[14:33:08][Step 4/4] Makefile:29: recipe for target 't0250-files-api.sh' failed
[14:33:08][Step 4/4] make[1]: Leaving directory '/teamcity/work/3144e2a06f62fd48/src/github.com/ipfs/go-ipfs/test/sharness'
[14:33:08][Step 4/4] Makefile:98: recipe for target 'test_sharness_expensive' failed
[14:33:08][Step 4/4] Process exited with code 2
[14:33:10][Step 4/4] Step sharness (Command Line) failed

I don't know what code 2 is, or why the recipe failed. I go to t0250-files-api.sh in my editor. I don't know what to look for, I don't see anything that looks like a doc fix.

What am I missing? How do you know what to look for in these tests outputs? What do you search for?

@RichardLitt RichardLitt removed the need/author-input Needs input from the original author label May 18, 2016
@Kubuxu
Copy link
Member

Kubuxu commented May 18, 2016

@RichardLitt
Copy link
Member Author

@Kubuxu Yes; that's what I copied above. But that text isn't helpful for me, because I am missing some context (which is what I am trying to find out). I see that 0250 failed - but why?

@Kubuxu
Copy link
Member

Kubuxu commented May 18, 2016

My link should go straight to a diff line: http://ci.ipfs.team:8111/viewLog.html?buildId=1966&buildTypeId=GoIpfs_CiTests&tab=buildLog&state=22239#_state=22239&focus=22237

0 bytes were expected, but command printed something, or in reverse.

@RichardLitt RichardLitt force-pushed the feature/add-default-to-files branch from 4f23a42 to 20492c6 Compare May 19, 2016 00:00
@Kubuxu Kubuxu force-pushed the feature/add-default-to-files branch from 20492c6 to b9fdd89 Compare May 19, 2016 19:11
Part of #2484

License: MIT
Signed-off-by: Jakub Sztandera <[email protected]>
@Kubuxu Kubuxu force-pushed the feature/add-default-to-files branch from b9fdd89 to 791b2cc Compare May 19, 2016 19:12
@Kubuxu
Copy link
Member

Kubuxu commented May 19, 2016

For some reason this fails here: http://ci.ipfs.team:8111/viewLog.html?buildId=2050&buildTypeId=GoIpfs_CiTests&tab=buildLog#_focus=22315

but I am not able to reproduce it:

 ~/git/go-ipfs   feature/add-default-to-files  cmd/ipfs/ipfs files read  /cat
Hello world
 ~/git/go-ipfs   feature/add-default-to-files  cmd/ipfs/ipfs files read  --count 0 /cat
 ~/git/go-ipfs   feature/add-default-to-files  

@Kubuxu Kubuxu added the help wanted Seeking public contribution on this issue label May 19, 2016
@whyrusleeping
Copy link
Member

@RichardLitt unless you object strongly, i'm going to close this one its mostly adding Default(false) to boolean options, and we have agreed elsewhere that this isnt necessary

@RichardLitt
Copy link
Member Author

@whyrusleeping There are a few things in this PR. Want me to split them out?

@whyrusleeping
Copy link
Member

@RichardLitt yeah, that would be great

@RichardLitt
Copy link
Member Author

Closing in favor of #3167 and #3168.

@RichardLitt RichardLitt removed the help wanted Seeking public contribution on this issue label Sep 19, 2016
@Kubuxu Kubuxu deleted the feature/add-default-to-files branch February 27, 2017 20:38
@asymmetric
Copy link
Contributor

its mostly adding Default(false) to boolean options, and we have agreed elsewhere that this isnt necessary

Can I ask why that decision was made? I'm a first-time user of the IPFS daemon and having some flags explain the default value, and some not, is confusing to me.

@whyrusleeping
Copy link
Member

@asymmetric the default value of any boolean option is false unless otherwise noted.

@asymmetric
Copy link
Contributor

OK, but then shouldn't it be so consistently?

As you can see below, some options (e.g. --init) specify a Default: false, whereas others (e.g. --migrate) don't. That's what was confusing for me.

IPFS version 0.4.10.

asymmetric@vps418113:~$ ipfs daemon --help
USAGE
  ipfs daemon - Run a network-connected IPFS node.

[...]

OPTIONS

  --init                         bool   - Initialize ipfs with default settings if not already initialized. Default: false.
  --mount                        bool   - Mounts IPFS to the filesystem. Default: false.
  --writable                     bool   - Enable writing objects (with POST, PUT and DELETE). Default: false.

  --unrestricted-api             bool   - Allow API access to unlisted hashes. Default: false.
  --disable-transport-encryption bool   - Disable transport encryption (for debugging protocols). Default: false.
  --enable-gc                    bool   - Enable automatic periodic repo garbage collection. Default: false.
  --manage-fdlimit               bool   - Check and raise file descriptor limits if needed. Default: true.
  --offline                      bool   - Run offline. Do not connect to the rest of the network but provide local API. Default: false.
  --migrate                      bool   - If true, assume yes at the migrate prompt. If false, assume no.
  --enable-pubsub-experiment     bool   - Instantiate the ipfs daemon with the experimental pubsub feature enabled.
  --enable-mplex-experiment      bool   - Add the experimental 'go-multiplex' stream muxer to libp2p on construction. Default: true.

@whyrusleeping
Copy link
Member

@asymmetric Yeah, it should be consistent. Anywhere that says "default false" should get fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/docs-ipfs Topic docs-ipfs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants