Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

feat: GetAppDetails function in templates/triggers #155

Merged
merged 4 commits into from
Jan 24, 2021

Conversation

paydaycay
Copy link
Contributor

Hi ! I suggest this PR to add function repo.GetAppDetails to templates/triggers to support loading application details.

Hope the code is clear enough because it's my first time in golang

#150

@CLAassistant
Copy link

CLAassistant commented Jan 22, 2021

CLA assistant check
All committers have signed the CLA.

@paydaycay
Copy link
Contributor Author

Hi @alexmt I fixed the problem with the tests (sorry I didn't write test but I'm not comfortable enough with golang to write tests) but I don't know how to resolve the problem with golangci-lint.
Someone already encountered the problem but I don't know if we are in same case.
Could you help me to fix this ?

@alexmt
Copy link
Collaborator

alexmt commented Jan 22, 2021

That looks very solid for a first contribution! Thank you @paydaycay ! looking

@alexmt
Copy link
Collaborator

alexmt commented Jan 22, 2021

Awesome! I have no comments for code, just formatting nitpicks.

Can you please run this command to sort imports:

find . -name \*.go | xargs -I {} goimports -w {} 

and one more to regenerate mocks

make generate

@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #155 (d31a4ef) into master (fdad4fe) will decrease coverage by 1.41%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #155      +/-   ##
==========================================
- Coverage   53.04%   51.63%   -1.42%     
==========================================
  Files          32       31       -1     
  Lines        1361     1435      +74     
==========================================
+ Hits          722      741      +19     
- Misses        508      558      +50     
- Partials      131      136       +5     
Impacted Files Coverage Δ
cmd/tools/context.go 16.66% <0.00%> (-1.41%) ⬇️
expr/repo/repo.go 22.00% <0.00%> (-14.67%) ⬇️
pkg/services/webhook.go 67.92% <0.00%> (-2.08%) ⬇️
shared/legacy/settings.go 53.52% <0.00%> (-1.87%) ⬇️
pkg/services/opsgenie.go 0.00% <0.00%> (ø)
pkg/templates/compiled.go
pkg/templates/service.go 62.50% <0.00%> (+8.65%) ⬆️
pkg/triggers/service.go 77.77% <0.00%> (+11.11%) ⬆️
pkg/services/slack.go 30.90% <0.00%> (+11.99%) ⬆️
pkg/services/services.go 24.32% <0.00%> (+24.32%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdad4fe...d31a4ef. Read the comment docs.

@paydaycay
Copy link
Contributor Author

Thanks @alexmt ! I hope it will be useful for the community. :)
I ran the commands and pushed

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@alexmt alexmt merged commit e3fb73b into argoproj-labs:master Jan 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants