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

feat(Variables): Recognize : in address to support output source #10208

Merged
merged 1 commit into from Nov 8, 2021

Conversation

pgrzesik
Copy link
Contributor

@pgrzesik pgrzesik commented Nov 8, 2021

Reported internally. It addresses missing support for all cases of output variable source. Ref: https://www.serverless.com/framework/docs/guides/output-variables

@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #10208 (8a7b0c3) into master (ecf2918) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 8a7b0c3 differs from pull request most recent head 6758237. Consider uploading reports for the commit 6758237 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10208      +/-   ##
==========================================
- Coverage   85.35%   85.34%   -0.01%     
==========================================
  Files         339      339              
  Lines       13831    13829       -2     
==========================================
- Hits        11805    11803       -2     
  Misses       2026     2026              
Impacted Files Coverage Δ
lib/configuration/variables/parse.js 98.95% <ø> (-0.01%) ⬇️

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 ecf2918...6758237. Read the comment docs.

@pgrzesik pgrzesik requested a review from medikoo November 8, 2021 15:01
medikoo
medikoo previously approved these changes Nov 8, 2021
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Looks great 👍

Still I think I would mark it a new feature of new resolver not a bug fix.

Current behavior was intentional, and new findings forced us to change the behavior

@pgrzesik pgrzesik force-pushed the recognize-colon-in-variable-address branch from db6598f to 6758237 Compare November 8, 2021 15:26
@pgrzesik pgrzesik changed the title fix(Variables): Recognize : in address to support output source feat(Variables): Recognize : in address to support output source Nov 8, 2021
@pgrzesik pgrzesik requested a review from medikoo November 8, 2021 15:41
Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

👍

@pgrzesik pgrzesik merged commit 723927f into master Nov 8, 2021
@pgrzesik pgrzesik deleted the recognize-colon-in-variable-address branch November 8, 2021 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants