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

docs: Add Sub function example to pseudo parameters doc. #8885

Merged
merged 2 commits into from Feb 4, 2021

Conversation

walery
Copy link
Contributor

@walery walery commented Feb 3, 2021

Update documentation for change added in #8279

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #8885 (39a26e2) into master (61dd3bd) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8885      +/-   ##
==========================================
- Coverage   87.72%   87.69%   -0.04%     
==========================================
  Files         266      266              
  Lines        9907     9907              
==========================================
- Hits         8691     8688       -3     
- Misses       1216     1219       +3     
Impacted Files Coverage Δ
lib/plugins/aws/provider.js 94.77% <ø> (ø)
lib/plugins/aws/deploy/lib/checkForChanges.js 95.68% <0.00%> (-2.59%) ⬇️

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 61dd3bd...c3ff60d. Read the comment docs.

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Thank you @walery, unfortunately I think the proposed example is misleading, as it's not going to be a valid part of serverless.yml if that was the intention. Maybe it would be better to use something like

functions:
  hello:
    handler: my-function.handler
    environment:
      var: !Sub arn:aws:logs:${AWS::Region}:${AWS::AccountId}:log-group:/aws/lambda/*:*:*'

Please let me know what do you think 🙇

@walery
Copy link
Contributor Author

walery commented Feb 4, 2021

Yes, I thought about it as well. Then I decided to use the same example as above.

Let me fix both examples 👍

@pgrzesik
Copy link
Contributor

pgrzesik commented Feb 4, 2021

Oh, I didn't notice that the example above is also wrong - if you could adjust them both that would be perfect. Thank you 🙇

The examples are now valid part of serverless.
See serverless#8885 discussion.
@walery
Copy link
Contributor Author

walery commented Feb 4, 2021

I've now adjusted both examples. Thank you @pgrzesik

Copy link
Contributor

@pgrzesik pgrzesik left a comment

Choose a reason for hiding this comment

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

Awesome job @walery - thanks for not only expanding docs but also improving old misleading example 🙇

@pgrzesik pgrzesik changed the title Add Sub function example to Pseudo Parameters doc. docs: Add Sub function example to pseudo parameters doc. Feb 4, 2021
@pgrzesik pgrzesik merged commit a11a43c into serverless:master Feb 4, 2021
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

2 participants