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

clarify types and link to Type Constraints pagedit types #33132

Merged
merged 3 commits into from
Sep 1, 2023

Conversation

briskt
Copy link
Contributor

@briskt briskt commented May 2, 2023

  • Add set to the types list on the types.mdx page.
  • Clarify relationship between complex types.
  • Link to Type Constraints page for more detailed information on types.

Note: this definition of set is copied from the Type Constraints page, though that language may not be clear in this context. Rather than risk making a technical error, I opted to stay with the same text.

Target Release

1.4.x

Draft CHANGELOG entry

N/A

@hashicorp-cla
Copy link

hashicorp-cla commented May 2, 2023

CLA assistant check
All committers have signed the CLA.

@crw
Copy link
Collaborator

crw commented May 3, 2023

Thanks for this submission! I have notified the docs team.

@briskt
Copy link
Contributor Author

briskt commented Aug 31, 2023

Should I continue to leave this PR open? There hasn't been any activity in several months.

@crw
Copy link
Collaborator

crw commented Sep 1, 2023

I think the docs team is a little backed up. Let me check in with them.

@judithpatudith judithpatudith added the tw-reviewed Technical Writing has reviewed this PR. label Sep 1, 2023
Co-authored-by: Judith Malnick <judith@hashicorp.com>
@crw crw added the 1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Sep 1, 2023
@crw crw merged commit 35baa30 into hashicorp:main Sep 1, 2023
6 of 7 checks passed
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

@judithpatudith, there's some confusion about type specifics here, should we roll this back or rewrite it?

* `map` (or `object`): a group of values identified by named labels, like
* `list`: a sequence of values, like `["us-west-1a", "us-west-1c"]`. Identify elements in a list with consecutive whole numbers, starting with zero.
* `set`: a collection of unique values that do not have any secondary identifiers or ordering.
* `map`: a group of values identified by named labels, like
Copy link
Member

Choose a reason for hiding this comment

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

object was removed from the definition here, but the example is an object and not a map

`{name = "Mabel", age = 52}`.

Strings, numbers, and bools are sometimes called _primitive types._ Lists/tuples and maps/objects are sometimes called _complex types,_ _structural types,_ or _collection types._
Strings, numbers, and bools are sometimes called _primitive types._ Lists and sets are forms
of tuples. Maps are a form of objects. Tuples and objects are sometimes called _complex types,_
Copy link
Member

@jbardin jbardin Sep 5, 2023

Choose a reason for hiding this comment

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

list and set are not forms of tuple, though homogeneous tuples can be converted to and from lists, and sets are unordered.

@judithpatudith
Copy link
Contributor

@jbardin great catch, sorry about that! I'd say overwrite it; do you have time to open the PR?

jbardin added a commit that referenced this pull request Sep 6, 2023
@crw
Copy link
Collaborator

crw commented Sep 7, 2023

We will want to backport the fix into 1.5 and 1.6 branches, just FYI.

@jbardin
Copy link
Member

jbardin commented Sep 8, 2023

@crw sorry, I forgot the backlinks, already done in #33831 and #33832

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged documentation tw-reviewed Technical Writing has reviewed this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants