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

Added fix for Internal transform failure on bad Condition syntax #908

Merged
merged 14 commits into from
May 8, 2019

Conversation

jadhavmanoj
Copy link
Contributor

Issue #892 :

Internal transform failure on bad Condition syntax:

Description of how you validated changes:

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-io
Copy link

codecov-io commented May 3, 2019

Codecov Report

Merging #908 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #908      +/-   ##
===========================================
+ Coverage    94.65%   94.66%   +<.01%     
===========================================
  Files           69       69              
  Lines         3011     3016       +5     
  Branches       559      561       +2     
===========================================
+ Hits          2850     2855       +5     
  Misses          85       85              
  Partials        76       76
Impacted Files Coverage Δ
samtranslator/sdk/resource.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd10d0e...0c0e289. Read the comment docs.

Copy link
Contributor

@jlhood jlhood left a comment

Choose a reason for hiding this comment

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

Really appreciate you taking the time to fix this bug! We reviewed and have some minor comments, but in verifying our comments, we ended up making the changes ourselves so we'll go ahead and push those commits and get this merged in. Thanks again!

samtranslator/sdk/resource.py Outdated Show resolved Hide resolved
samtranslator/sdk/resource.py Outdated Show resolved Hide resolved
samtranslator/sdk/resource.py Outdated Show resolved Hide resolved
samtranslator/sdk/resource.py Outdated Show resolved Hide resolved
@jadhavmanoj
Copy link
Contributor Author

Really appreciate you taking the time to fix this bug! We reviewed and have some minor comments, but in verifying our comments, we ended up making the changes ourselves so we'll go ahead and push those commits and get this merged in. Thanks again!

Thanks for the review & comments @jlhood and @keetonian

samtranslator/sdk/resource.py Outdated Show resolved Hide resolved
@keetonian
Copy link
Contributor

@jadhavmanoj no problem! Thank you for fixing this!

@keetonian keetonian merged commit 71ab9b8 into aws:develop May 8, 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.

None yet

4 participants