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

Add TFE Backwards Compatibility contrib documentation #1011

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

sebasslash
Copy link
Contributor

@sebasslash sebasslash commented Aug 17, 2023

Description

Add docs related to TFE backwards compatibility.

Fixes #996

Remember to:

Testing plan

  1. Describe how to replicate
  2. the conditions
  3. under which your code performs its purpose,
  4. including example Terraform configs where necessary.

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.

$ TESTARGS="-run TestAccTFEWorkspace" make testacc

...

@sebasslash sebasslash requested a review from a team as a code owner August 17, 2023 23:05
@sebasslash sebasslash force-pushed the sebasslash/tf-8382_tfe-version-constraints branch 2 times, most recently from d8a1153 to 66695e8 Compare August 18, 2023 15:35
Copy link
Contributor

@rberecka rberecka left a comment

Choose a reason for hiding this comment

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

Documentation was clear and helpful!!

@sebasslash sebasslash force-pushed the sebasslash/tf-8382_tfe-version-constraints branch 2 times, most recently from ca5c23d to 281a6f1 Compare August 18, 2023 20:34
@sebasslash
Copy link
Contributor Author

This PR closes #996

Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

I hate to continue to push back on this but I still think we could get away indefinitely with not adding this type of condition to this repo. Why not just if tmAccess.ProjectAccess != nil { ... if tmAccess.WorkspaceAccess != nil { ? Not only does it look better, but it doesn't create any protocol dependency. 🤷

docs/backwards-compatibility.md Show resolved Hide resolved
docs/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/backwards-compatibility.md Outdated Show resolved Hide resolved
docs/backwards-compatibility.md Outdated Show resolved Hide resolved
tfe/provider.go Outdated Show resolved Hide resolved
tfe/provider.go Outdated Show resolved Hide resolved
@sebasslash sebasslash force-pushed the sebasslash/tf-8382_tfe-version-constraints branch from 281a6f1 to ebcc272 Compare August 21, 2023 17:02
@sebasslash
Copy link
Contributor Author

Yeah implicit checks are probably the simplest option here (and maybe I should call it out as the recommended approach). I like this kind of check for instances where nil checking a field isn't sufficient. It also serves as a historical record that's easier to determine "What version of TFE supports this" w/o having to dig through commit history and retrace the steps.

@sebasslash sebasslash force-pushed the sebasslash/tf-8382_tfe-version-constraints branch 2 times, most recently from 209753a to f4ce7c6 Compare August 23, 2023 16:33
This document outlines how to handle unsupported TFE versions, TFE only behavior,
and updating the documentation to reflect supported TFE versions.
@sebasslash sebasslash force-pushed the sebasslash/tf-8382_tfe-version-constraints branch from f4ce7c6 to 20c9986 Compare August 24, 2023 21:36
@sebasslash sebasslash changed the title Add tfeVersionGTE() and supporting docs Add TFE Backwards Compatibility contrib documentation Aug 24, 2023
Comment on lines +21 to +23
"settings": tmAccess.ProjectAccess.ProjectSettingsPermission,
"teams": tmAccess.ProjectAccess.ProjectTeamsPermission,
}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"settings": tmAccess.ProjectAccess.ProjectSettingsPermission,
"teams": tmAccess.ProjectAccess.ProjectTeamsPermission,
}}
"settings": tmAccess.ProjectAccess.ProjectSettingsPermission,
"teams": tmAccess.ProjectAccess.ProjectTeamsPermission,
}}

@sebasslash sebasslash merged commit 7952a76 into main Aug 24, 2023
9 checks passed
@sebasslash sebasslash deleted the sebasslash/tf-8382_tfe-version-constraints branch August 24, 2023 23:38
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.

v0.48.0 crash for tfe_team_project_access and tfe_team_access
3 participants