Small markups on API usage #7
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
normand1
reviewed
Sep 11, 2020
README.md
Outdated
| @@ -6,11 +6,11 @@ These are the required headers for each request: | |||
|
|
|||
| * "**User-Agent**" - Please identify the system/product you are using to make this request. | |||
|
|
|||
| * "**X-Auth-Date**" - The current unix epoch time as a string. 5 minute window. | |||
| * "**X-Auth-Date**" - The current unix epoch time as a string. 5 minute window. If accessing from Python, use only the integer portion of the time value. Note that JavaScript's time value from a Date's getTime() is compatible, but needs to be scaled down by 1,000 before also being converted to an integer. | |||
There was a problem hiding this comment.
This is true, but I don't think this kind of language specific definition should be present in this generic documentation at the top-level. Maybe there's a way to phrase this that would be more generic?
"The current unix epoch time (seconds since the Epoch) as a string"
That would hopefully signal to users they should use the rounded to the nearest second representation of epoch time which would solve the issue for both JS and Python users.
normand1
reviewed
Sep 11, 2020
README.md
Outdated
|
|
||
| * "**X-Auth-Key**" - Your API key string. | ||
|
|
||
| * "**Authorization**" - A SHA-1 hash of the X-Auth-Key, the corresponding key secret and the X-Auth-Date value concatenated as a string. The resulting hash should be encoded as hexadecimal value. | ||
| * "**Authorization**" - An SHA-1 hash of the X-Auth-Key, the corresponding key, secret and the X-Auth-Date value concatenated as a string. The resulting hash should be encoded as a hexadecimal value, two digits per byte, using lower case letters for the hex digits "a" through "f". |
There was a problem hiding this comment.
- If you pronounce SHA-1 like the Iranian Monarch (as I tend to) then the original grammar here is correct I believe.
- Adding the
ais a good idea. - I'm not sure that the final addition is necessary, I haven't run into a problem with using SHA-1 yet, is that an issue you had?
|
Hi,
I don't think this kind of language specific definition should be present in this generic documentation
I'm an old standards guy--the generic goal resonates, but the world has really focused down to a few languages which are basically the modern latin. If you talk Python and JS, all the C/Rust/C#/Lisp/etc. people know exactly what you mean. I included JS because it's the 1,000 pound gorilla, and their coders tend to be some of the weakest. But don't quote me.
OTOH, you could ignore me and spend the time fleshing out some Python and JS examples, and that might be the clearest way to document it. At a minimum, I suggest making it clear it's an integer value of seconds (the Python people will for sure be lobbing float's at you).
Thanks,
Andy
September 10, 2020 5:22 PM, "David Norman" <notifications@github.com (mailto:notifications@github.com?to=%22David%20Norman%22%20<notifications@github.com>)> wrote:
@normand1 commented on this pull request.
------------------------------------
In README.md (#7 (comment)):
@@ -6,11 +6,11 @@ These are the required headers for each request: * "**User-Agent**" - Please identify the system/product you are using to make this request. -* "**X-Auth-Date**" - The current unix epoch time as a string. 5 minute window. +* "**X-Auth-Date**" - The current unix epoch time as a string. 5 minute window. If accessing from Python, use only the integer portion of the time value. Note that JavaScript's time value from a Date's getTime() is compatible, but needs to be scaled down by 1,000 before also being converted to an integer.
This is true, but I don't think this kind of language specific definition should be present in this generic documentation at the top-level. Maybe there's a way to phrase this that would be more generic?
"The current unix epoch time (seconds since the Epoch) as a string"
That would hopefully signal to users they should use the rounded to the nearest second representation of epoch time which would solve the issue for both JS and Python users.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#7 (review)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAM22KJ5DB2G54SHJYHX5GDSFFUS7ANCNFSM4RF4PRKQ).
|
|
#1 OK! :)
#2 thanks
#3 it wasn't a hassle I had here, but it is a hassle I've had in past lives
Thanks again,
Andy
September 10, 2020 5:29 PM, "David Norman" <notifications@github.com (mailto:notifications@github.com?to=%22David%20Norman%22%20<notifications@github.com>)> wrote:
@normand1 commented on this pull request.
------------------------------------
In README.md (#7 (comment)):
* "**X-Auth-Key**" - Your API key string. -* "**Authorization**" - A SHA-1 hash of the X-Auth-Key, the corresponding key secret and the X-Auth-Date value concatenated as a string. The resulting hash should be encoded as hexadecimal value. +* "**Authorization**" - An SHA-1 hash of the X-Auth-Key, the corresponding key, secret and the X-Auth-Date value concatenated as a string. The resulting hash should be encoded as a hexadecimal value, two digits per byte, using lower case letters for the hex digits "a" through "f".
* If you pronounce SHA-1 like the Iranian Monarch (as I tend to) then the original grammar here is correct I believe.
* Adding the a is a good idea.
* I'm not sure that the final addition is necessary, I haven't run into a problem with using SHA-1 yet, is that an issue you had?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#7 (review)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAM22KNOBP2C6Q5U4PLLZ2TSFFVNRANCNFSM4RF4PRKQ).
|
I absolutely agree with this, I ran into the same issue. If you wanted to make those couple tweaks to the Pull Request I'd be happy to give this a thumbs up! |
|
Ok, let me be sure which couple of tweaks. int-ness of time, got it. Case of hex? And "as [a] hex...". |
Yep, those make the most sense to me as a user. I feel like the other info is good too, but would probably fit better into one of the platform specific repos or in one of the platform specific examples below. |
normand1
approved these changes
Sep 11, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, feel free to use/edit/ignore in any combination which pleases you!