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

remove tour command from ipfs #4123

Merged
merged 1 commit into from
Aug 16, 2017

Conversation

sherodtaylor
Copy link

I thought I'd take a stab at my first contribution to ipfs :) #4117

@sherodtaylor sherodtaylor force-pushed the feature/tour/remove-tour branch from 5de6016 to 19e32eb Compare August 6, 2017 03:37
@Kubuxu
Copy link
Member

Kubuxu commented Aug 6, 2017

I think you will also have to remove connected sharness tests.

@sherodtaylor sherodtaylor force-pushed the feature/tour/remove-tour branch 3 times, most recently from 3bcf818 to aefe9f3 Compare August 7, 2017 13:31
@sherodtaylor
Copy link
Author

@Kubuxu what sharness test? I can't find a reference to the tour command?

@Kubuxu
Copy link
Member

Kubuxu commented Aug 7, 2017

 …/ipfs/go-ipfs  master  cd test/sharness             
 …/test/sharness  master  ag tour
t0110-gateway.sh
143:for cmd in "add" "block/put" "bootstrap" "config" "dht" "diag" "dns" "get" "id" "mount" "name/publish" "object/put" "object/new" "object/patch" "pin" "ping" "refs/local" "repo" "resolve" "stats" "swarm" "tour" "file" "update" "version" "bitswap"; do

@Kubuxu
Copy link
Member

Kubuxu commented Aug 7, 2017

You can also go here: https://ci.ipfs.team/blue/organizations/jenkins/go-ipfs/detail/PR-4123/8/pipeline?start=0#step-80-log-0

and search for not ok that isn't marked as TODO known breakage

License: MIT
Signed-off-by: Sherod Taylor <sherodtaylor@gmail.com>
@sherodtaylor sherodtaylor force-pushed the feature/tour/remove-tour branch from 086e0ea to bc75d3b Compare August 9, 2017 14:15
@sherodtaylor
Copy link
Author

@Kubuxu fixed the sharness test but one build is failing due to issue #4055 and code climate failed because bindata.go is generated. Should I ignore because i don't want to edit the generated file.

@Kubuxu
Copy link
Member

Kubuxu commented Aug 9, 2017

Ignore the codeclimate on bindata.

@sherodtaylor
Copy link
Author

sherodtaylor commented Aug 9, 2017

@Kubuxu is this good to go then?

@Kubuxu Kubuxu requested a review from a user August 9, 2017 19:55
@sherodtaylor
Copy link
Author

any update on this @lgierth

@Kubuxu
Copy link
Member

Kubuxu commented Aug 11, 2017

SGTM but I would like to someone to re-review the bindata.

@whyrusleeping
Copy link
Member

?This LGTM, could @Kubuxu or @lgierth regen the go-bindata stuff on their own? Would be great to verify all that binary looking nonsense

@sherodtaylor
Copy link
Author

Is this good to go or are we waiting on another review?

@whyrusleeping
Copy link
Member

@sherodtaylor Its nearly good to go, I was hoping someone else could verify the bindata generated here. I'll just go ahead and do it now.

@whyrusleeping
Copy link
Member

Alright, verified this locally. The bindata is correct. Thanks @sherodtaylor !

@whyrusleeping whyrusleeping merged commit 9ea02e9 into ipfs:master Aug 16, 2017
@sherodtaylor
Copy link
Author

@whyrusleeping np! Is there anything else I can jump on?

@whyrusleeping
Copy link
Member

@sherodtaylor theres always something to be done :) I did a quick scan through, and this issue might be something you could tackle: #4052. If that looks intimidating then really anything in the issue tracker is fair game, reviewing other peoples code is also one of the most helpful things anyone can do

Hop into #ipfs-dev on freenode IRC if you have questions

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.

None yet

3 participants