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

Remove unnecessary homedir module #88

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

RedbackThomson
Copy link
Contributor

os.UserHomeDir is supported in go1.12+. We no longer need a third party module to load the home directory.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-bot ack-bot requested review from a-hilaly and jaypipes June 10, 2021 20:07
@vijtrip2
Copy link
Contributor

/approve

@RedbackThomson
Copy link
Contributor Author

/hold

Might still need it. Golang is not good at expanding tilde

@ack-bot ack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@RedbackThomson
Copy link
Contributor Author

/unhold

It wasn't golang that sucked at expanding tilde, it was the fact we were explicitly passing tilde as the command line argument

@ack-bot ack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@a-hilaly
Copy link
Member

Thanks @RedbackThomson ! :)

/approve

@a-hilaly
Copy link
Member

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Jun 10, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [A-Hilaly,RedbackThomson,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 8988516 into aws-controllers-k8s:main Jun 10, 2021
@RedbackThomson RedbackThomson deleted the remove-homedir branch June 10, 2021 22:33
ack-prow bot pushed a commit that referenced this pull request Sep 27, 2023
#462)

**Issue #**
This issue arises from the code changes in lambda-controller for[ PR #88](aws-controllers-k8s/lambda-controller#88)

**Summary**
Support of injecting fields in existing structures: CustomNestedFields

**Description**
The code in this PR helps in accepting a custom nestedField defined using `type` as a part of its parentField. In the issue mentioned above when we inject a new field `S3SHA256` into `Code` struct, instead of nesting it under `Code` the generator created a separate field named `CodeS3SHA256`. 

The new field `S3SHA256` is defined like this in `generator.yaml` file:
```
Function:
    fields:
      Code.S3SHA256:
        type: string
```

After applying the changes in this PR, the field `S3SHA256` is properly injected as a part of `Code` struct.

```
type FunctionCode struct {
	ImageURI *string `json:"imageURI,omitempty"`
	S3Bucket *string `json:"s3Bucket,omitempty"`
	S3BucketRef     *ackv1alpha1.AWSResourceReferenceWrapper `json:"s3BucketRef,omitempty"`
	S3Key           *string                                  `json:"s3Key,omitempty"`
	S3ObjectVersion *string                                  `json:"s3ObjectVersion,omitempty"`
	S3SHA256        *string                                  `json:"s3SHA256,omitempty"`
	ZipFile         []byte                                   `json:"zipFile,omitempty"`
} 
```

This PR contains code implementation to inject Nested fields into the Spec or Status struct and has respective model-test to validate the changes.

**Acknowledgment**
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants