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.

Sign up for GitHub

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

Jump to bottom

docs: decapitalize IPFS where possible #2911

Merged
merged 2 commits into from
Nov 2, 2016
Merged

docs: decapitalize IPFS where possible #2911

merged 2 commits into from
Nov 2, 2016

Conversation

RichardLitt
Copy link
Member

Tried to check all instances of IPFS and make sure they werent referring to the CLI tool. See #2910.

License: MIT
Signed-off-by: Richard Littauer richard.littauer@gmail.com

@RichardLitt RichardLitt added topic/docs-ipfs Topic docs-ipfs need/review Needs a review labels Jun 27, 2016
Kubuxu
Kubuxu reviewed Jun 27, 2016
View reviewed changes
core/commands/cat.go
@@ -20,7 +20,7 @@ var CatCmd = &cmds.Command{
},

Arguments: []cmds.Argument{
cmds.StringArg("ipfs-path", true, true, "The path to the IPFS object(s) to be outputted.").EnableStdin(),
cmds.StringArg("ipfs-path", true, true, "The path to the ipfs object(s) to be outputted.").EnableStdin(),
Copy link
Member

Choose a reason for hiding this comment

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

In this case it should probably capital. It is about IPFS object.

@Kubuxu Kubuxu removed their assignment Jun 27, 2016
@RichardLitt
Copy link
Member Author

@Kubuxu updated

@Kubuxu
Copy link
Member

Kubuxu commented Jun 28, 2016

SGTM but now you have merge conflict with master, you need to rebase.

@RichardLitt RichardLitt force-pushed the feature/ifps-capitalize branch 2 times, most recently from a37beb5 to 3dfcc45 Compare June 30, 2016 13:30
@RichardLitt
Copy link
Member Author

@Kubuxu rebased

whyrusleeping
whyrusleeping reviewed Jul 1, 2016
View reviewed changes
core/commands/block.go
@@ -43,10 +43,10 @@ multihash.

var blockStatCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Print information of a raw IPFS block.",
Tagline: "Print information of a raw ipfs block.",
Copy link
Member

Choose a reason for hiding this comment

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

why is this one lowercase while the surrounding two are capitalized?

@whyrusleeping
Copy link
Member

@RichardLitt youre gonna have to fix some of the tests. You changed print statements that we test the output of

@whyrusleeping whyrusleeping added need/author-input Needs input from the original author and removed need/review Needs a review labels Aug 18, 2016
@RichardLitt RichardLitt self-assigned this Aug 19, 2016
@RichardLitt
Copy link
Member Author

@whyrusleeping I tried to find the failing tests that are relevant to this, and I'm not seeing any. Can you help me?

@RichardLitt RichardLitt added need/review Needs a review and removed need/author-input Needs input from the original author labels Sep 1, 2016
@RichardLitt RichardLitt removed their assignment Sep 1, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Sep 1, 2016

@RichardLitt t0020 - ipfs init should not run while daemon is running and ipfs cat no repo message looks good

When you open test console on circle, search for not ok.

@RichardLitt
Copy link
Member Author

@Kubuxu I saw both of those: I don't know how to fix those particular issues. What code should I change to make that pass? How would I know? Can you please help me by listing exactly what you would do to fix one of these, here - I just don't know the workflow.

@whyrusleeping
Copy link
Member

@RichardLitt take a look at the code for the test that is failing. They are in the test/sharness folder. The string next to the 'not ok' messages tells you which test. Look at the code being executed by the test and see why it would be failling.

@RichardLitt
Copy link
Member Author

Ok:

  1. I looked at ipfs cat no repo message looks good
  2. I saw that it is making a command, and piping it to cat_test_out.
  3. I replicate that by running:
test_expect_success "ipfs cat fails" '
    export IPFS_PATH="$(pwd)/.ipfs" &&
    test_must_fail ipfs cat Qmaa4Rw81a3a1VEx4LxB7HADUAXvZFhCoRdBzsMZyZmqHD 2> cat_fail_out

as

export IPFS_PATH="$(pwd)/.ipfs" &&
ipfs cat Qmaa4Rw81a3a1VEx4LxB7HADUAXvZFhCoRdBzsMZyZmqHD

It looks fine to me, and no different on master, where I run the same thing. What am I missing?

@whyrusleeping whyrusleeping added the status/in-progress In progress label Sep 14, 2016
@RichardLitt
Copy link
Member Author

@whyrusleeping Can you help me with my question above?

@RichardLitt RichardLitt removed their assignment Sep 19, 2016
@RichardLitt
Copy link
Member Author

@Kubuxu?

@whyrusleeping
Copy link
Member

@RichardLitt The issue here is that you changed the capitalization of ipfs. in some tests
in some tests we are expecting to see it lowercase, but now its capitalized.

You will have to update these tests and change the capitalization the same way you did in the code.

While on this branch, cd into the test/sharness folder and run make deps. Then set export TEST_NO_FUSE=1 to skip running the fuse tests (they likely won't run on your machine anyways), and export TEST_VERBOSE=1 to see verbose test output. After that, you can run make or run any of the tests individually with ./test-name.sh -v.

@RichardLitt
Copy link
Member Author

Updated. Only failures were in the docker tests, which were unrelated - not sure why those aren't working.

@RichardLitt @whyrusleeping
docs: decapitalize IPFS where possible
9aaa743 
Tried to check all instances of IPFS and make sure they werent referring to the CLI tool. See #2910.

License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
@RichardLitt @whyrusleeping
Changed so only explicit ipfs cli commands are lowercased
9843e86 
License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
@whyrusleeping whyrusleeping merged commit 437cc7f into master Nov 2, 2016
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Nov 2, 2016
@whyrusleeping whyrusleeping deleted the feature/ifps-capitalize branch November 2, 2016 20:20
@ghost ghost mentioned this pull request Dec 23, 2016
Closed
ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Apr 7, 2022
@whyrusleeping
Merge pull request ipfs#2911 from ipfs/feature/ifps-capitalize
43833d7 
docs: decapitalize IPFS where possible
hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@whyrusleeping
Merge pull request ipfs/kubo#2911 from ipfs/feature/ifps-capitalize
b3304c4 
docs: decapitalize IPFS where possible

This commit was moved from ipfs/kubo@437cc7f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
need/review Needs a review topic/docs-ipfs Topic docs-ipfs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RichardLitt @Kubuxu @whyrusleeping