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

CORE-1120: completed implementation of new notifications service. #2

Merged
merged 27 commits into from Oct 5, 2020

Conversation

slr71
Copy link
Member

@slr71 slr71 commented Sep 18, 2020

No description provided.

…d ID validation to the `/v2/messages/{id}` endpoint
@slr71 slr71 force-pushed the v1-api branch 2 times, most recently from 5a9f95e to b843caf Compare September 18, 2020 01:41
db/listings.go Show resolved Hide resolved
@slr71 slr71 marked this pull request as ready for review September 18, 2020 02:03
Copy link
Member

@ianmcorvidae ianmcorvidae left a comment

Choose a reason for hiding this comment

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

Things look pretty good to me. I have a few comments, mostly ideas on concision. I was pretty confused by one section of code that I'm hoping you can help clarify. I also still need to learn how labstack/echo works better, so I may do another pass on this, but don't wait on me to do that or anything, heh.

db/listings.go Outdated Show resolved Hide resolved
db/listings.go Show resolved Hide resolved
db/listings.go Show resolved Hide resolved
db/listings.go Show resolved Hide resolved
… fields in the response body for the /v2/messages endpoint
@slr71
Copy link
Member Author

slr71 commented Oct 5, 2020

I'm going to merge this PR now. Thanks for the review!

@slr71 slr71 merged commit 5ce4663 into cyverse-de:master Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants