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 Group Resource #3

Merged
merged 24 commits into from
Jan 19, 2024
Merged

Conversation

GomathiselviS
Copy link
Contributor

@GomathiselviS GomathiselviS commented Dec 6, 2023

This PR adds group resource. Refer https://issues.redhat.com/browse/ACA-1158

terraform {
  required_providers {
    aap = {
      source = "ansible/aap"
    }
  }
}

provider "aap" {
  host     = "https://controller.xxx.xyz/"
  username = "xxxx"
  password = "xxxx"
  insecure_skip_verify = true
}

resource "aap_group" "sample" {
  inventory_id = 1
  name = "tf_group" 
  variables = jsonencode({"ansible_network_os": "ios"})
}

@GomathiselviS GomathiselviS marked this pull request as draft December 6, 2023 19:46
@GomathiselviS GomathiselviS marked this pull request as ready for review December 14, 2023 21:53
go.mod Outdated Show resolved Hide resolved
internal/provider/group_resource.go Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
@GomathiselviS GomathiselviS changed the title WIP: Add Group Resource Add Group Resource Jan 4, 2024
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Show resolved Hide resolved
internal/provider/group_resource.go Show resolved Hide resolved
internal/provider/group_resource_test.go Outdated Show resolved Hide resolved
examples/resources/group/main.tf Outdated Show resolved Hide resolved
golangci-lint Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource_test.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

LGTM, Just lint issues to fix

@GomathiselviS
Copy link
Contributor Author

LGTM, Just lint issues to fix

Hi @abikouo Can you help me fix this lint failure. The failure happens in the job_resource_test.go

@GomathiselviS GomathiselviS requested a review from abikouo January 17, 2024 14:36
@gravesm
Copy link
Member

gravesm commented Jan 18, 2024

Hi @abikouo Can you help me fix this lint failure. The failure happens in the job_resource_test.go

This seems to me like a bad linting error. It's claiming that lines 450-480 of job_resource_test.go are duplicates of lines 515-545. They are not, as one is testing idempotence without triggers and the other is testing idempotence with triggers. They are very similar, but clearly not duplicates.

IMO, running anything more than just go fmt on tests is not worth it, since tests often are written in ways that code analyzers are likely to flag. I would look at the linters we have running and see if there's a configuration you can use that gets it to ignore this. I'm not sure why this wasn't flagged in the job resource PR.

@GomathiselviS
Copy link
Contributor Author

Hi @abikouo Can you help me fix this lint failure. The failure happens in the job_resource_test.go

This seems to me like a bad linting error. It's claiming that lines 450-480 of job_resource_test.go are duplicates of lines 515-545. They are not, as one is testing idempotence without triggers and the other is testing idempotence with triggers. They are very similar, but clearly not duplicates.

IMO, running anything more than just go fmt on tests is not worth it, since tests often are written in ways that code analyzers are likely to flag. I would look at the linters we have running and see if there's a configuration you can use that gets it to ignore this. I'm not sure why this wasn't flagged in the job resource PR.

Based on the information provided in https://github.com/mibk/dupl?tab=readme-ov-file#dupl, it appears that the golangci-lint will generate duplicate code (dupl errors) for the test cases in job_resource_test.go

internal/provider/group_resource.go Outdated Show resolved Hide resolved
internal/provider/group_resource.go Show resolved Hide resolved
internal/provider/group_resource.go Outdated Show resolved Hide resolved
@alinabuzachis alinabuzachis mentioned this pull request Jan 19, 2024
@GomathiselviS GomathiselviS merged commit 556ab69 into ansible:main Jan 19, 2024
2 checks passed
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.

5 participants