Skip to content

Conversation

atikhono
Copy link

No description provided.

@atikhono
Copy link
Author

@theckman Could you please have a look?

@theckman
Copy link
Collaborator

theckman commented Apr 11, 2017

@atikhono Happy to look. Do you have more information around what failure this resolves? What kind of resource call triggers it?

Our test kitchen tests haven't been failing, so it appears we are missing a test case there. Can you please enhance the test kitchen tests to fail if this were to regress?

Edit: Sorry for the edit. Along the lines of writing the tests, can you please enhance the commit message to detail what failure this is fixing? When someone runs git-blame in the future, it's helpful to know why changes were made to the source code. If you could squash the test addition, and additional commit details, in to one commit I'd really appreciate it!

Copy link
Collaborator

@theckman theckman left a comment

Choose a reason for hiding this comment

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

I'm going to set this to reviewed, with changes requested while waiting on the following:

  • more details being added to the initial commit
  • tests being added to confirm that this failure mode is fixed

@theckman
Copy link
Collaborator

Ahem, okay so now I've had some caffeine and noticed #113. Sorry about that! That issue does show the head of the failure itself, which makes sense. I think a test case being written for this PR would solve my inquiry about what kind of resource call triggers it.

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.

2 participants