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

🍀 Proposal: Plugin argocd Enhancement (sub-task of #513) #514

Closed
daniel-hutao opened this issue May 18, 2022 · 4 comments
Closed

🍀 Proposal: Plugin argocd Enhancement (sub-task of #513) #514

daniel-hutao opened this issue May 18, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest hacktoberfest only

Comments

@daniel-hutao
Copy link
Member

What would you like to add? Why is this needed?

See #513 for the details

@daniel-hutao daniel-hutao added the enhancement New feature or request label May 18, 2022
@daniel-hutao daniel-hutao changed the title 🍀 Proposal: Plugin enhancement - argocd 🍀 Proposal: Plugin Enhancement - argocd May 18, 2022
@daniel-hutao daniel-hutao changed the title 🍀 Proposal: Plugin Enhancement - argocd 🍀 Proposal: (sub-task with #513) Plugin Enhancement - argocd May 18, 2022
@daniel-hutao daniel-hutao changed the title 🍀 Proposal: (sub-task with #513) Plugin Enhancement - argocd 🍀 Proposal: (sub-task with #513) Plugin Enhancement with argocd May 18, 2022
@daniel-hutao daniel-hutao changed the title 🍀 Proposal: (sub-task with #513) Plugin Enhancement with argocd 🍀 Proposal: Plugin argocd Enhancement (sub-task with #513) May 18, 2022
@daniel-hutao daniel-hutao changed the title 🍀 Proposal: Plugin argocd Enhancement (sub-task with #513) 🍀 Proposal: Plugin argocd Enhancement (sub-task of #513) May 18, 2022
@IronCore864
Copy link
Member

Adding more details about this issue:

Be Brave, Be Imaginative

Anything that isn't following the clean code best practice, refactor it.

Any feature you think would add value to end-users, add it.

Any configuration refactors / documentation updates that would improve the end users' quality of life, do it.

Be brave when deleting unnecessary code, and be imaginative when adding new features.

Possible Improvements: Config Refactor

The current options section is like this:

  options:
    create_namespace: true
    repo:
      name: argo
      url: https://argoproj.github.io/argo-helm
    chart:
      chart_name: argo/argo-cd
      release_name: argocd
      namespace: argocd
      wait: true
      timeout: 5m
      upgradeCRDs: true
      values_yaml: |
        controller:
          service: 
            port: 8080
        redis:
          image:
            tag: 6.2.6-alpine3.15

There are a couple of possible improvements:

  1. Provide default values for repo.name and repo.url. I.E., the repo section should be optional. The reason is that most users would install ArgoCD from the default/official repo.
  2. chart_name might be a little bit misleading. It's rather "CHART_REPO/CHARTNAME`. A better name is needed.
  3. Provide default values for chart.release_name, chart.namespace, chart.wait, chart.upgradeCRDs, and chart.timeout.
  4. Provide a better example in the documentation, especially for the values_yaml section. The current example doesn't seem quite necessary (like Redis version, and service port 8080, not what normal users would override the default values from the chart).

Possible Improvements: Code Improvement

  1. Code: internal/pkg/plugin/argocd/argocd.go is empty, not necessary maybe. But since the dtm develop command creates this file, maybe the better thing to do here is to move the logic from the state.go to this file.
  2. Functions refactor. Make them easier to read and easier to test.
  3. Add unit tests wherever possible.

@IronCore864 IronCore864 added the good first issue Good for newcomers label May 25, 2022
@IronCore864
Copy link
Member

This issue might be too big for a good first issue, but we still want to give it a try since it's mainly refactoring and improving.

@daniel-hutao daniel-hutao added the help wanted Extra attention is needed label Jul 1, 2022
@jxs1211
Copy link
Contributor

jxs1211 commented Jul 6, 2022

I will give it a try @daniel-hutao

@steinliber
Copy link
Contributor

steinliber commented Aug 3, 2022

TODO:

@steinliber steinliber self-assigned this Aug 3, 2022
@daniel-hutao daniel-hutao added hacktoberfest hacktoberfest only and removed help wanted Extra attention is needed labels Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest hacktoberfest only
Projects
None yet
Development

No branches or pull requests

4 participants