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

Add version --all option #2790

Merged
merged 3 commits into from
Jun 2, 2016
Merged

Add version --all option #2790

merged 3 commits into from
Jun 2, 2016

Conversation

Kubuxu
Copy link
Member

@Kubuxu Kubuxu commented Jun 1, 2016

Resolves #2789

License: MIT
Signed-off-by: Jakub Sztandera kubuxu@protonmail.ch

License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@chriscool
Copy link
Contributor

This is great and works for me:

> ipfs version --all
go-ipfs version: 0.4.2-0bcfc49
Repo version: 3
System version: amd64/linux
Golang version: go1.5.3

Maybe you could add a sharness test?

Thanks!

@chriscool chriscool mentioned this pull request Jun 2, 2016
@RichardLitt
Copy link
Member

Works nicely on the API.

🐕  curl -i "http://localhost:5001/api/v0/version?all"
HTTP/1.1 200 OK
Access-Control-Allow-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Access-Control-Expose-Headers: X-Stream-Output, X-Chunked-Output, X-Content-Length
Content-Type: application/json
Server: go-ipfs/0.4.2
Trailer: X-Stream-Error
Vary: Origin
Date: Thu, 02 Jun 2016 09:04:06 GMT
Transfer-Encoding: chunked

{"Version":"0.4.2","Commit":"0bcfc49","Repo":"3","System":"amd64/darwin","Golang":"go1.5.3"}

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 2, 2016

@RichardLitt all version calls will return those values as all options for version command are text formatting only.

I will add few tests later.

@Kubuxu Kubuxu self-assigned this Jun 2, 2016
@RichardLitt
Copy link
Member

Ah right. :)

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 2, 2016

Added tests also moved the regression tests further apart number wise.

License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
@chriscool
Copy link
Contributor

LGTM, thanks!

@whyrusleeping
Copy link
Member

LGTM

@whyrusleeping whyrusleeping merged commit 994109c into master Jun 2, 2016
@whyrusleeping whyrusleeping deleted the feature/version-all branch June 2, 2016 19:23
@ligi
Copy link

ligi commented Jun 13, 2016

just thinking: isn't this changing the API?
currently curl "http://localhost:5001/api/v0/version?all"
returns {"Version":"0.4.2","Commit":"1654bbf","Repo":"3"}

afterwards it will return ( with the same api version )
{"Version":"0.4.2","Commit":"0bcfc49","Repo":"3","System":"amd64/darwin","Golang":"go1.5.3"}

Perhaps it is time for v1 ( could also include the local refs change from #2803)

@Kubuxu
Copy link
Member Author

Kubuxu commented Jun 13, 2016

This isn't even breaking change (API is just extended).
Also it is v0 version of the API it is and will be modified. I think if we were to version the API at this stage we would go with v01, v02 and so on, but it isn't planned yet.

refs local changes the API but as it can be seen nobody was previously using it as they would report its incorrect behaviour.

@ligi
Copy link

ligi commented Jun 13, 2016 via email

@jbenet
Copy link
Member

jbenet commented Aug 26, 2016

... changes the API but as it can be seen nobody was previously using it as they would report its incorrect behaviour.

Never assume that. Ever.

@jbenet
Copy link
Member

jbenet commented Aug 26, 2016

@ligi thank you very much for flagging this and #2803 as breaking changes, it got me to take a closer look at them (#2812 (comment))

@Kubuxu
Copy link
Member Author

Kubuxu commented Aug 26, 2016

Is adding fields to JSON structure a breaking change?

@Kubuxu Kubuxu removed their assignment Aug 26, 2016
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.

6 participants