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

argocd diff #3

Merged
merged 36 commits into from
Apr 25, 2024
Merged

argocd diff #3

merged 36 commits into from
Apr 25, 2024

Conversation

Oded-B
Copy link
Collaborator

@Oded-B Oded-B commented Apr 12, 2024 •
edited
Loading

Description

Optionally add ArgoCD "diff" comment on PRs
Light refactoring of the promotion logic to allow ArgoCD functionality to re-use of generateListOfRelevantComponents

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change
  • Refactor
  • Documentation
  • Other (please describe)

Checklist

  • I have read the contributing guidelines
  • Existing issues have been referenced (where applicable)
  • I have verified this change is not present in other open pull requests
  • Functionality is documented
  • All code style checks pass
  • New code contribution is covered by automated tests
  • All new and existing tests pass

@Oded-B
Copy link
Collaborator Author

Oded-B commented Apr 17, 2024

/publish

@github-actions GitHub Actions
Copy link

oci-image-publish-on-comment: succeeded ✅
https://github.com/commercetools/telefonistka/actions/runs/8721693331
ghcr.io/commercetools/telefonistka:argocd-diff
ghcr.io/commercetools/telefonistka:sha-002b806

@Oded-B
Copy link
Collaborator Author

Oded-B commented Apr 18, 2024

/publish

@github-actions GitHub Actions
Copy link

oci-image-publish-on-comment: succeeded ✅
https://github.com/commercetools/telefonistka/actions/runs/8734027016
ghcr.io/commercetools/telefonistka:argocd-diff
ghcr.io/commercetools/telefonistka:sha-c8992f0

@Oded-B
Copy link
Collaborator Author

Oded-B commented Apr 18, 2024

/publish

@github-actions GitHub Actions
Copy link

oci-image-publish-on-comment: succeeded ✅
https://github.com/commercetools/telefonistka/actions/runs/8738715361
ghcr.io/commercetools/telefonistka:argocd-diff
ghcr.io/commercetools/telefonistka:sha-12536e9

@Oded-B
Copy link
Collaborator Author

Oded-B commented Apr 22, 2024

/publish

@github-actions GitHub Actions
Copy link

oci-image-publish-on-comment: succeeded ✅
https://github.com/commercetools/telefonistka/actions/runs/8781663728
ghcr.io/commercetools/telefonistka:argocd-diff
ghcr.io/commercetools/telefonistka:sha-6777665

@Oded-B Oded-B marked this pull request as ready for review April 22, 2024 10:20
hnnsgstfssn
hnnsgstfssn reviewed Apr 22, 2024
View reviewed changes
Copy link

@hnnsgstfssn hnnsgstfssn left a comment

Choose a reason for hiding this comment

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

Generally a pretty massive change.

Since we're aiming to make progress and get this into a working state I'd be ok with going ahead and address refactors and improvements later.

README.md Outdated Show resolved Hide resolved
docs/installation.md Outdated Show resolved Hide resolved
docs/installation.md Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
internal/pkg/githubapi/github.go Show resolved Hide resolved
internal/pkg/githubapi/promotion.go Show resolved Hide resolved
internal/pkg/githubapi/promotion.go Outdated Show resolved Hide resolved
internal/pkg/githubapi/github.go Outdated Show resolved Hide resolved
internal/pkg/argocd/argocd.go Outdated Show resolved Hide resolved
internal/pkg/githubapi/github.go Show resolved Hide resolved
Oded-B and others added 5 commits April 23, 2024 09:02
@Oded-B
Add PR diff image to README.md
edd33d1
@Oded-B
Adress PR comments:
e0b9831 
Document env var default values
Remove dead code
Named return value in multiple function
Renaming/spliting of some vars
Switched from  map[relevantComponent]bool) to desribe a set  map[relevantComponent]struct{})
@Oded-B
Address more PR comments:
b344d82 
Add some error handeling
Document some functions
@Oded-B
fix lint issue
4c0fda7
@Oded-B
Lint fix
b6d3e87
yzdann
yzdann reviewed Apr 23, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
Oded-B and others added 2 commits April 24, 2024 10:07
hnnsgstfssn
hnnsgstfssn approved these changes Apr 24, 2024
View reviewed changes
Copy link

@hnnsgstfssn hnnsgstfssn left a comment

Choose a reason for hiding this comment

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

After discussion, we've talked about the points brought up and current state does what we need. We defer additional improvements and refactors until later.

@Oded-B Oded-B merged commit 8a752da into main Apr 25, 2024
5 checks passed
@Oded-B Oded-B mentioned this pull request Apr 25, 2024
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yzdann yzdann yzdann left review comments

@hnnsgstfssn hnnsgstfssn hnnsgstfssn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Oded-B @hnnsgstfssn @yzdann