From 6ebb6b968a59971d903d7630fad4fe9bfc6cc83c Mon Sep 17 00:00:00 2001 From: IBRAHIM Kabiru A <38470660+IbrahimAndela@users.noreply.github.com> Date: Tue, 3 Mar 2020 07:15:10 +0100 Subject: [PATCH 1/2] Create feedback_note.md --- feedback_note.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 feedback_note.md diff --git a/feedback_note.md b/feedback_note.md new file mode 100644 index 0000000..f6fcaf6 --- /dev/null +++ b/feedback_note.md @@ -0,0 +1,36 @@ +# Feedback + +## Language + Score : 4/5 ==> Exceeds Expectations + +1. I'm not sure what is meant here https://github.com/gideon-maina/rss-processor/blob/master/fetchrss/fetch.go#L95 +but this is necessary to ensure we capture and are using that current loop value in the go routines, else we would be +using stale values since in the for loop, we cannot gurantee when each go routine would be executed. + +2. Commenting of functions doesn't follow standard Go comments e.g This should start with the function name +https://github.com/gideon-maina/rss-processor/blob/master/fetchrss/fetch.go#L64 . Also linters could be setup for such +cases. + +3. Uses a basic http handler/router which is just perfect for this project but I believe he's aware of other handler/router +setups where multiple routes with different HTTP methods are expected so we don't end up check things like HTTP +Method over and over again. https://github.com/gideon-maina/rss-processor/blob/master/serverss/server.go#L38 + +4. I do not see any reason why we need to handle http Requests in new Go routines. Each request to the server is +already handled in a new Go routine in Go's http lib. +https://github.com/gideon-maina/rss-processor/blob/master/serverss/server.go#L44 + +5. Uses Go Modules + +## Architecture and Design +Score : 3/5 ==> Meets Expectations + +1. Follows more of a CLI approach to building a Web serivce. + +## Testing & Security +Score: 4/5 ==> Exceeds Expectations + + 1. Demonstrates use of Tokens to ensure endpoints are protected. + 2. We could implement RateLimiting to prevent potential DDOS attacks. (I know this is an overkill, but would be nice if it was written as TODO in Readme ;)) + + # Summary + ## Overall Average Rating = 3.7/5 From 2687dcb9cee69b0feffa608cceb2e2ca52b3acba Mon Sep 17 00:00:00 2001 From: IBRAHIM Kabiru A <38470660+IbrahimAndela@users.noreply.github.com> Date: Tue, 3 Mar 2020 09:15:37 +0100 Subject: [PATCH 2/2] Update feedback_note.md --- feedback_note.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/feedback_note.md b/feedback_note.md index f6fcaf6..d65b9fc 100644 --- a/feedback_note.md +++ b/feedback_note.md @@ -33,4 +33,4 @@ Score: 4/5 ==> Exceeds Expectations 2. We could implement RateLimiting to prevent potential DDOS attacks. (I know this is an overkill, but would be nice if it was written as TODO in Readme ;)) # Summary - ## Overall Average Rating = 3.7/5 + ## Overall Average Rating = 3.7/5 ==> Meets Expectations