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

Feature: Sum Function #24666

Merged
merged 5 commits into from
Apr 15, 2020
Merged

Feature: Sum Function #24666

merged 5 commits into from
Apr 15, 2020

Conversation

noahmercado
Copy link
Contributor

@noahmercado noahmercado commented Apr 14, 2020

Reference: #17239

@alisdair
Continuation of closed PR -> #24356

This feature adds an additional collection function, sum. sum takes a list, set, or tuple of numbers and returns the sum of all elements.

[X] Early validation of idea and implementation plan: Terraform's core is complicated enough that there are often several ways to implement something, each of which has different implications and tradeoffs. Working through a plan of attack with the team before you dive into implementation will help ensure that you're working in the right direction.

[X] Unit tests: Terraform's core is covered by hundreds of unit tests at several different layers of abstraction. Generally the best place to start is with a "Context Test". These are higher level test that interact end-to-end with most of Terraform's core. They are divided into test files for each major action (plan, apply, etc.). Getting a failing test is a great way to prove out a bug report or a new enhancement. With a context test in place, you can work on implementation and lower level unit tests. Lower level tests are largely context dependent, but the Context Tests are almost always part of core work.

[X] Documentation updates: If the core change involves anything that needs to be reflected in our documentation, you can make those changes in the same PR. The Terraform website source is in this repo and includes instructions for getting a local copy of the site up and running if you'd like to preview your changes.

[X] Well-formed Code: Do your best to follow existing conventions you see in the codebase, and ensure your code is formatted with go fmt. (The Travis CI build will fail if go fmt has not been run on incoming code.) The PR reviewers can help out on this front, and may provide comments with suggestions on how to improve the code.

@ghost ghost added the sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK label Apr 14, 2020
@noahmercado
Copy link
Contributor Author

@alisdair turns out I'm not able to assign you the PR but figured tagging in a comment would suffice :)

@alisdair alisdair self-requested a review April 14, 2020 19:45
@alisdair alisdair self-assigned this Apr 14, 2020
@alisdair alisdair marked this pull request as ready for review April 14, 2020 19:45
@alisdair
Copy link
Contributor

Thanks! Closing and reopening to hopefully trigger a CI build 🤞

@alisdair alisdair closed this Apr 14, 2020
@alisdair alisdair reopened this Apr 14, 2020
@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #24666 into master will increase coverage by 0.00%.
The diff coverage is 90.90%.

Impacted Files Coverage Δ
lang/funcs/collection.go 78.41% <90.47%> (+0.98%) ⬆️
lang/functions.go 92.62% <100.00%> (+0.06%) ⬆️
terraform/node_resource_plan.go 91.59% <0.00%> (-1.69%) ⬇️
dag/marshal.go 52.27% <0.00%> (-1.14%) ⬇️
terraform/evaluate.go 41.70% <0.00%> (-0.66%) ⬇️
states/statefile/version4.go 55.32% <0.00%> (+0.25%) ⬆️
backend/remote/backend_common.go 55.63% <0.00%> (+0.70%) ⬆️

Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

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

This is great, thank you for your contribution! 🎉

@alisdair alisdair merged commit d4d8812 into hashicorp:master Apr 15, 2020
paddycarver pushed a commit that referenced this pull request Apr 23, 2020
The sum function takes a list or set of numbers and returns the sum of those
numbers.
@ghost
Copy link

ghost commented May 16, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants