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

feat(manifests) resource kind is case-insensitive (internal refactor) #412

Merged
merged 11 commits into from
Feb 6, 2022

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Dec 11, 2021

Depends on #388 , #414, #415, #418, #453, #456, #457, #479, #480, #481, #482 (will be rebased once all merged).

This PR is a refactor around the plugins to factorize common behaviors of resources (e.g. sources, conditions and targets) when possible, by using struct composition (https://golangbot.com/inheritance/) to look-a-like OO Object-Oriented's inheritance.

The expected outcomes are:

  • (end-users) the type of resource, specified by the attribute kind, is now case un-sensitive
  • (contributors) A single place to initialize the list of plugins (centralized unmarshaling for instance)
  • (contributors) improved code convention for plugins (meaning constant behavior)
  • (contributors) code base easier to browse as resources plugins are separated from scms plugins and "utils/generic" plugins

I recommend you to review this PR commit by commit (without checking the 4 commits from #482 of course) since it is refactoring!
It introduces the following changes (1 per commit):

  • Introduces a new package resource with a ResourceConfig struct. This package defines the common behavior that any plugin resource should implement.
  • Factorize the unmarshalling away from source, condition and target, into this new resource package. It allows a centralized place to list the resource plugin + delegating parsing logic on each resource plugin.
  • This "centralization" allows to define the kind of each resource to be case insensitive 🥳
  • The changelog retrieval is now implemented by all the resources. If not implemented, then it returns an empty string. It makes the changelog retrieval way easier to implement/change and makes it usable with the internal state of resources.
  • Refactor the directory + package structure of plugin. It makes it really clear which package implements which kind of "behavior" in updatecli
$ tree -L 1 pkg/plugins
pkg/plugins
├── resources # plugins that implements the interface resource (e.g. a source/condition/target)
├── scms # plugins that implements the SCM interface (git / github)
└── utils # other plugins which behaviors are reused across other plugins but not in core

Test

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

make test
make test-short
make test-e2e

Additional Information

Please note that this PR lays the foundation for #373, #413 and #497 (deprecating unused fileds)

@dduportal dduportal force-pushed the refactor-resources branch 2 times, most recently from bae73e9 to 5ec8508 Compare December 12, 2021 13:45
@dduportal dduportal force-pushed the refactor-resources branch 6 times, most recently from 39f0ca6 to 4ee417e Compare January 8, 2022 13:38
@dduportal dduportal force-pushed the refactor-resources branch 3 times, most recently from 8b20daa to 658a3c0 Compare January 12, 2022 20:20
@dduportal dduportal force-pushed the refactor-resources branch 2 times, most recently from ad80119 to fdcb75e Compare January 18, 2022 18:53
@dduportal dduportal force-pushed the refactor-resources branch 3 times, most recently from 37596cf to 4f2c224 Compare January 23, 2022 11:36
@dduportal dduportal force-pushed the refactor-resources branch 3 times, most recently from 3f4c908 to 0b77c13 Compare January 23, 2022 21:50
@dduportal dduportal force-pushed the refactor-resources branch 2 times, most recently from c706690 to c96e98c Compare February 1, 2022 17:49
dduportal and others added 8 commits February 3, 2022 18:56
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
… plugin resources

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>
Introduces 3 sub directories: resources, scms and utils

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Co-authored-by: Hervé Le Meur <91831478+lemeurherve@users.noreply.github.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@olblak
Copy link
Member

olblak commented Feb 4, 2022

@dduportal Well done, it's a nice pull request. It's interesting to see that you implemented in ways hesitated to do. I hesitated a Like the same interface for source, condition, target, but in the end, your approach makes things clearer.
Considering that it's a very big refactoring, I would like to take additional time for testing.

@olblak
Copy link
Member

olblak commented Feb 5, 2022

I think before merging this PR we should also clean up the CONTRIBUTING

@olblak
Copy link
Member

olblak commented Feb 5, 2022

So far I didn't identify any regression 🎉

@dduportal dduportal enabled auto-merge (rebase) February 6, 2022 19:21
@dduportal dduportal merged commit 476c0fd into updatecli:main Feb 6, 2022
@dduportal dduportal deleted the refactor-resources branch February 6, 2022 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup condition Modify condition resource enhancement New feature or request resource-aws-ami Resource of kind AWS AMI resource-docker Resource of kind Docker Image resource-dockerdigest Resource of kind dockerDigest resource-dockerfile Resource of kind Dockerfile resource-file Resource of kind File resource-githubRelease Resource of kind GitHub Release resource-gittag Resource of kind Git Tag resource-helmchart Resource of kind Helm Chart resource-jenkins Resource of kind Jenkins resource-mavenrepository Resource of kind Maven Repository resource-shell Resource of kind Shell resource-yaml Resource of kind YAML scm-git SCM of kind "Git" scm-github SCM of type GiHhub source target Related to updatecli target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants