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

Fix staticcheck warnings | Resolve all but one TODO comments #191

Closed
wants to merge 10 commits into from

Conversation

taimoorgit
Copy link
Contributor

@taimoorgit taimoorgit commented Jan 21, 2024

Issues

Part of: https://github.com/OpsLevel/team-platform/issues/186

Changelog

  • Fix U1000's - unused stuff
  • Fix ST1005 - don't capitalize error messages
  • Change if cond == true to become if cond
  • Service lifecycle_alias and tier_alias should actually be mandatory
  • The existingTags var in reconcileTags() is never used, staticcheck complained about this.
  • Remove TODO in team - managed_aliases bug does not affect Team.
  • Remove another TODO in Service because the provider doesn't support ValidateFunc on a type list.
  • Make a changie entry

Tophatting

I tophatted the reconcileTags() change to make sure lifecycle_alias and tier_alias were actually required and to make sure that existingTags wasn't a used variable. It worked.

resource "opslevel_service" "redbull" {
  name = "__ testing the tags in tf"
  lifecycle_alias = "pre-alpha"
  tier_alias = "tier_4"
  # step 1: create with no tags OR create with first tags 0,1 and skip to step 3
  # step 2: add tags 0,1 and terraform apply
  # step 3: remove tag 0, add tag 2 and terraform apply
  # all steps worked - diffs generated by terraform looked good
  tags = ["hello:world", "was:here", "removed:first"]
}

For managed_aliases tophatting on Team resource:

resource "opslevel_team" "redbull_racing" {
  name = "redbull racing"
  responsibilities = "idk"
  # step 1: use only this alias - terraform apply
  # step 2: run terraform apply again
  # terraform does not complain and try to add in the slug (redbull_racing)
  # i can still get the team in CLI using 'rb' or slug 'redbull_racing'
  # TL;DR managed_aliases is not necessary on the team resource despite
  # the API providing it because it doesn't suffer from the slug glitch.
  aliases = ["rb"]
  parent = "platform"
}

@taimoorgit taimoorgit added the enhancement New feature or request label Jan 21, 2024
@taimoorgit taimoorgit self-assigned this Jan 21, 2024
Copy link

codecov bot commented Jan 21, 2024

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Comparison is base (212f2d0) 44.75% compared to head (5140d39) 46.12%.

Files Patch % Lines
opslevel/resource_opslevel_team.go 0.00% 7 Missing ⚠️
opslevel/common.go 25.00% 6 Missing ⚠️
opslevel/datasource_opslevel_filter.go 0.00% 2 Missing ⚠️
opslevel/datasource_opslevel_integration.go 0.00% 2 Missing ⚠️
opslevel/datasource_opslevel_lifecycle.go 0.00% 2 Missing ⚠️
opslevel/datasource_opslevel_rubric_category.go 0.00% 2 Missing ⚠️
opslevel/datasource_opslevel_rubric_level.go 0.00% 2 Missing ⚠️
opslevel/datasource_opslevel_tier.go 0.00% 2 Missing ⚠️
opslevel/resource_opslevel_check_custom_event.go 0.00% 2 Missing ⚠️
...level/resource_opslevel_check_service_ownership.go 0.00% 2 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #191      +/-   ##
==========================================
+ Coverage   44.75%   46.12%   +1.36%     
==========================================
  Files          71       70       -1     
  Lines        5865     5691     -174     
==========================================
  Hits         2625     2625              
+ Misses       3235     3061     -174     
  Partials        5        5              

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

@taimoorgit taimoorgit changed the title Fix staticcheck warnings Fix staticcheck warnings | Resolve some TODO comments Jan 21, 2024
@@ -101,26 +100,11 @@ func resourceService() *schema.Resource {
ForceNew: false,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
// ValidateFunc: validateServiceTags, // TODO: Not Yet Supported
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This TODO item isn't valid because ValidateFunc does not work on schema.TypeList. Source:

// ValidateFunc is honored only when the schema's Type is set to TypeInt,
// TypeFloat, TypeString, TypeBool, or TypeMap. It is ignored for all other types.
ValidateFunc [SchemaValidateFunc](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/helper/[email protected]#SchemaValidateFunc)

https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/helper/[email protected]#Schema.ValidateFunc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There already exists examples of adding tags to services that the user can refer to. If the list is invalid they will get an API error anyways.

Examples

@taimoorgit taimoorgit changed the title Fix staticcheck warnings | Resolve some TODO comments Fix staticcheck warnings | Resolve all but one TODO comments Jan 21, 2024
@taimoorgit taimoorgit added go Pull requests that update Go code code_quality Improves code quality and removed enhancement New feature or request labels Jan 21, 2024
@taimoorgit taimoorgit closed this Feb 26, 2024
@davidbloss davidbloss deleted the ta/rly-small-fixes branch July 16, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code_quality Improves code quality go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant