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

Replace "Managed by Terraform" in our docs #2611

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Nov 12, 2024

Fixes #2610

@iwahbe iwahbe self-assigned this Nov 12, 2024
Copy link

codecov bot commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.79%. Comparing base (e0fa71e) to head (9674437).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2611      +/-   ##
==========================================
- Coverage   62.79%   62.79%   -0.01%     
==========================================
  Files         389      389              
  Lines       51938    51939       +1     
==========================================
- Hits        32615    32613       -2     
- Misses      17515    17517       +2     
- Partials     1808     1809       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iwahbe iwahbe requested a review from a team November 12, 2024 15:35
@iwahbe iwahbe enabled auto-merge (squash) November 13, 2024 15:37
Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

Typos and nites but LGTM otherwise. Thank you!

@@ -265,6 +265,36 @@ func TestApplyEditRules(t *testing.T) {
expected: []byte("# Configuration Reference"),
phase: info.PostCodeTranslation,
},
{
// Found in azuredevops: https://github.com/pulumi/pulumi-terraform-bridge/issues/2610
name: `Replaces "Managed by Terraform" with "Manged by Pulumi"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: `Replaces "Managed by Terraform" with "Manged by Pulumi"`,
name: `Replaces "Managed by Terraform" with "Managed by Pulumi"`,

`),
},
{
name: "Does not replace \"Managed by Terraform\" with \"Manged by Pulumi\" (require quotes)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
name: "Does not replace \"Managed by Terraform\" with \"Manged by Pulumi\" (require quotes)",
name: "Does not replace \"Managed by Terraform\" with \"Managed by Pulumi\" without quotes",

This reads a bit clearer to me. WDYT? Or maybe elaborate in the comment.

name: "Does not replace \"Managed by Terraform\" with \"Manged by Pulumi\" (require quotes)",
// It's harder to tell if Managed by Terraform would sense to replace, so we don't do it for now.
//
// We don't have a canonical example where this does not work.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "does not work" mean here?
Perhaps link a followup issue here that considers adding this? Since presumably if we do do so, this test would fail.

},
{
name: "Does not replace \"Managed by Terraform\" with \"Manged by Pulumi\" (require quotes)",
// It's harder to tell if Managed by Terraform would sense to replace, so we don't do it for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// It's harder to tell if Managed by Terraform would sense to replace, so we don't do it for now.
// It's harder to tell if Managed by Terraform would make sense to replace, so we don't do it for now.

@iwahbe iwahbe merged commit 77577d0 into master Nov 13, 2024
17 checks passed
@iwahbe iwahbe deleted the iwahbe/2610/replace-manged-by-terraform branch November 13, 2024 21:10
guineveresaenger added a commit that referenced this pull request Nov 14, 2024
Follow-up to #2611, which I didn't realize was set to automerge.
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v3.96.0.

@ericrudder
Copy link
Member

ericrudder commented Nov 22, 2024 via email

@iwahbe
Copy link
Member Author

iwahbe commented Nov 22, 2024

@ericrudder AFAIK these were all covered in #2619. I took another look at this PR, and couldn't find any typos still present in master.

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.

Azure DevOps example says "Managed by Terraform"
4 participants