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

Fix time format RFC3339 in version #283 #287

Merged
merged 1 commit into from
Jul 18, 2019
Merged

Conversation

u5surf
Copy link
Contributor

@u5surf u5surf commented Jul 17, 2019

Fixes #283
I added time.Format(time.RFC3339) in version

@CLAassistant
Copy link

CLAassistant commented Jul 17, 2019

CLA assistant check
All committers have signed the CLA.

@masci
Copy link
Contributor

masci commented Jul 17, 2019

@u5surf thanks for the PR, my apologies but you got caught right in the middle of an invasive merge, can you rebase on master and solve the conflicts? 🙇‍♂️

@u5surf u5surf force-pushed the issue-283 branch 4 times, most recently from 2d09f28 to 453f23d Compare July 17, 2019 15:11
@mastrolinux
Copy link
Contributor

mastrolinux commented Jul 17, 2019

The PR now perfectly fixes the standard output format, it does not solve yet the --format json case.

$ ./arduino-cli version 
arduino-cli Version: 0.3.7-alpha.preview Commit:  BuildDate: 2019-07-17T15:24:18Z
$ ./arduino-cli version --format json
{
  "Application": "arduino-cli",
  "VersionString": "0.3.7-alpha.preview",
  "Commit": "",
  "BuildDate": "2019-07-17T15:27:41.7405288Z"
}

Do you like to update the PR to completely fix #283 ?
Thanks for your help @u5surf !

@masci
Copy link
Contributor

masci commented Jul 17, 2019

We could add a ToJSON() method to the VersionInfo struct so we keep the format definition in one place

@u5surf
Copy link
Contributor Author

u5surf commented Jul 17, 2019

@mastrolinux I updated the PR to fix the both of format.
@masci Thx reviewing. After all, I implemented the new type rfc3339Time which has the string formatter and JsonMarshal function.

Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

Love it! Waiting for @mastrolinux to confirm the format is what we want but code LGTM

@masci masci merged commit d876ac9 into arduino:master Jul 18, 2019
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.

Wrong Date format in version output
4 participants