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.

Sign up for GitHub

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

Jump to bottom

feat: add default repo config for argo #853

Merged
merged 1 commit into from
Jul 18, 2022
Merged

feat: add default repo config for argo #853

merged 1 commit into from
Jul 18, 2022

Conversation

homily707
Copy link
Contributor

@homily707 homily707 commented Jul 14, 2022 •
edited
Loading

Signed-off-by: homily707 homily707@outlook.com

Pre-Checklist

Note: please complete ALL items in the following checklist.

  • I have read through the CONTRIBUTING.md documentation.
  • My code has the necessary comments and documentation (if needed).
  • I have added relevant tests

Description

add default repo name and url config for argo

Related Issues

#514

New Behavior (screenshots if needed)

@homily707 homily707 requested a review from a team as a code owner July 14, 2022 14:33
@homily707
Copy link
Contributor Author

this pr is a fix for pr851, thanks for the advice of @steinliber, please check it out~

steinliber
steinliber reviewed Jul 15, 2022
View reviewed changes
Copy link
Contributor

@steinliber steinliber left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Code location need to discuss

internal/pkg/plugin/argocd/state.go Show resolved Hide resolved
aFlyBird0
aFlyBird0 requested changes Jul 15, 2022
View reviewed changes
Copy link
Member

@aFlyBird0 aFlyBird0 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! Welcome to DevStream! 👏 @homily707

internal/pkg/plugin/argocd/argocd.go Outdated Show resolved Hide resolved
@aFlyBird0
Copy link
Member

Good job, all checks passed including commit message and DCO check! @homily707

@daniel-hutao daniel-hutao assigned steinliber and unassigned iyear Jul 15, 2022
aFlyBird0
aFlyBird0 reviewed Jul 16, 2022
View reviewed changes
internal/pkg/plugin/argocd/argocd.go Outdated Show resolved Hide resolved
internal/pkg/plugin/argocd/update.go Outdated Show resolved Hide resolved
@homily707
feat: add default repo config for argo
622ca90 
Signed-off-by: homily707 <homily707@outlook.com>
@homily707 homily707 requested a review from aFlyBird0 July 16, 2022 08:01
aFlyBird0
aFlyBird0 approved these changes Jul 16, 2022
View reviewed changes
Copy link
Member

@aFlyBird0 aFlyBird0 left a comment

Choose a reason for hiding this comment

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

Good Job! @homily707

@aFlyBird0 aFlyBird0 removed their assignment Jul 16, 2022
@IronCore864
Copy link
Member

Functions in this file internal/pkg/plugin/argocd/argocd.go can be moved to a better place because they aren't specific to argocd only. Merging for now, will improve later soon @steinliber

@IronCore864 IronCore864 merged commit 6ae1b4c into devstream-io:main Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aFlyBird0 aFlyBird0 aFlyBird0 approved these changes

@IronCore864 IronCore864 IronCore864 approved these changes

@steinliber steinliber Awaiting requested review from steinliber

@daniel-hutao daniel-hutao Awaiting requested review from daniel-hutao

Assignees

@steinliber steinliber

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@homily707 @aFlyBird0 @IronCore864 @steinliber @daniel-hutao @iyear