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

chore: refactor resource dockerfile #453

Merged
merged 8 commits into from
Jan 11, 2022

Conversation

dduportal
Copy link
Contributor

While working on #390 and #412, the need to refactor the dockerfile resource was raised.

This PR does not change the functionnality of the resource: it's why the label says "chore", except for the resulting commit messages (that were not really correct).

It introduces the following changes:

  • Fix the example to allow end to end testing from the root of the repository
  • Apply the same conventions as other refactored resources:
    • Define a Spec type on the package with only the public attributes
    • Add a New initializer method
  • Improve Developer experience:
    • Full unit test with mocked injected dependencies (no more file to be read/written on the FS while running tests!)
    • Increased test coverage that helped cleaning up unused struct attributes

Test

To test this pull request, you can run the following commands:

go build -o ./dist/updatecli && ./dist/updatecli diff --config examples/updatecli.generic/dockerfile

Additional Information

Tradeoff

Potential improvement

@dduportal dduportal added chore condition Modify condition resource resource-dockerfile Resource of kind Dockerfile target Related to updatecli target labels Jan 9, 2022
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
…ndency injection

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Copy link
Member

@olblak olblak left a comment

Choose a reason for hiding this comment

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

It looks good to me
🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore condition Modify condition resource resource-dockerfile Resource of kind Dockerfile target Related to updatecli target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants