Refactor job resource #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Alina Buzachis <[email protected]>
Signed-off-by: Alina Buzachis <[email protected]>
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis
force-pushed
the
refactoring/job/resource
branch
from
d570883 to
6e3a930
Compare
abikouo
requested changes
Feb 16, 2024
Signed-off-by: Alina Buzachis <[email protected]>
abikouo
approved these changes
Feb 16, 2024
mandar242
approved these changes
Feb 19, 2024
hakbailey
approved these changes
Feb 19, 2024
internal/provider/job_resource.go
Outdated
| @@ -41,7 +38,27 @@ func (r *JobResource) Metadata(_ context.Context, req resource.MetadataRequest, | |||
| resp.TypeName = req.ProviderTypeName + "_job" | |||
| } | |||
|
|
|||
| // Schema defines the schema for the resource. | |||
| // Configure adds the provider configured client to the data source. | |||
| func (d *JobResource) Configure(_ context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { | |||
There was a problem hiding this comment.
This seems to alternate between d and r as the JobResource variable throughout this file. I would pick one for consistency...probably r since it's referring to a resource (I think the d originally came from the inventory data source).
internal/provider/job_resource.go
Outdated
| } | ||
|
|
||
| func (r JobResource) CreateJob(data JobResourceModelInterface) diag.Diagnostics { | ||
| func (r *JobResource) CreateJob(data *JobResourceModel) diag.Diagnostics { |
There was a problem hiding this comment.
What if we called this function LaunchJob since that's really what it is doing? That would also help differentiate it from the Create function, and would make it extra clear that both Create and Update launch a job.
internal/provider/job_resource.go
Outdated
| // jobResourceModel maps the resource schema data. | ||
| type jobResourceModel struct { | ||
| // Job AAP API model | ||
| type JobAPIModel struct { |
There was a problem hiding this comment.
Similar to the group resource refactor, I would consider reorganizing this file so that each type is followed by its functions.
alinabuzachis
force-pushed
the
refactoring/job/resource
branch
from
7cae37e to
bdc35cd
Compare
Signed-off-by: Alina Buzachis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor job resource