这是indexloc提供的服务，不要输入任何密码
Skip to content
This repository was archived by the owner on Aug 23, 2024. It is now read-only.

[VOID] Conditional requests #291

Closed
wants to merge 2 commits into from
Closed

[VOID] Conditional requests #291

wants to merge 2 commits into from

Conversation

JacopoDaeli
Copy link
Contributor

Follow #266.

@JacopoDaeli JacopoDaeli changed the title Add individual conditional etag [WIP] Pass conditional "etag header" to individual requests Oct 8, 2017
@JacopoDaeli JacopoDaeli mentioned this pull request Oct 8, 2017
Closed
pksunkara
pksunkara reviewed Oct 9, 2017
View reviewed changes
src/octonode/cmd.coffee Outdated
# Requiring modules

# Initiate class
class Cmd
Copy link
Owner

Choose a reason for hiding this comment

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

Rename file and this to Base

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🌮

pksunkara
pksunkara reviewed Oct 9, 2017
View reviewed changes
src/octonode/client.coffee
@@ -41,6 +41,11 @@ class Client
if @token and typeof @token == 'string'
@requestDefaults.headers.Authorization = "token " + @token

# Add http conditional "etag"
conditional: (etag) ->
Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add documentation on how to use this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🌮

pksunkara
pksunkara suggested changes Oct 9, 2017
View reviewed changes
Copy link
Owner

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

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

Minor fixes needed

@jeffpearce jeffpearce mentioned this pull request Oct 9, 2017
Closed
@JacopoDaeli
Copy link
Contributor Author

@pksunkara should we bump the npm version as well?

@pksunkara
Copy link
Owner

Sure, go ahead with that.

@JacopoDaeli
Copy link
Contributor Author

Should go for 0.9.0 right? Or should we consider this as patch and go for 0.8.1?

@pksunkara
Copy link
Owner

Minor version is better.

@JacopoDaeli
Copy link
Contributor Author

👍

@JacopoDaeli JacopoDaeli changed the title [WIP] Pass conditional "etag header" to individual requests Conditional requests Oct 9, 2017
@JacopoDaeli
Copy link
Contributor Author

@pksunkara changes done. What do you think?

@pksunkara
Copy link
Owner

Hey, Sorry about the misunderstanding. I meant 0.8.1. Not 0.9.0. Could you correct it and squash the last commit? Thanks. Will merge and publish immediately once thats done.

@JacopoDaeli
Copy link
Contributor Author

@pksunkara I updated the package.json file 👍 . You can merge!

@pksunkara
Copy link
Owner

squash the last commit

Please squash the last 3 commits into a single one.

pksunkara and others added 2 commits October 11, 2017 09:39
@pksunkara @jdaeli-godaddy
# This is a combination of 4 commits.
9733e50 
# This is the 1st commit message:

Version bump v0.8.0

# This is the commit message #2:

Add individual conditional etag

# This is the commit message #3:

Fix client

Minor fixes + bump npm version to 0.9.0

Update README

Update package.json

# This is the commit message #4:

Fix client
@jdaeli-godaddy
Fix
d39ec9e
@JacopoDaeli
Copy link
Contributor Author

JacopoDaeli commented Oct 11, 2017 •
edited
Loading

closing this. opening a new one. I broke something :)

@JacopoDaeli JacopoDaeli deleted the JacopoDaeli/individualHTTPConditional branch October 11, 2017 16:48
@JacopoDaeli JacopoDaeli changed the title Conditional requests [VOID] Conditional requests Oct 11, 2017
@JacopoDaeli
Copy link
Contributor Author

@pksunkara #292
This should be ok!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pksunkara pksunkara pksunkara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JacopoDaeli @pksunkara @jdaeli-godaddy