first version#216
Conversation
|
Nice use of user prompt in ‘new peeps’ form ‘Please enter a new peep below:’ App has also been secured with the use of exec_params code is well laid out & easy to read - unnecessary files have been removed to make the files clear (such as the pre-given test page spec) Naming clear for files such as view/add |
| get '/test' do | ||
| 'Test page' | ||
|
|
||
| # get '/' do |
There was a problem hiding this comment.
I would probably try to avoid leaving commented out code in your code base, either you need that redirect or you don't
| # redirect '/chitter/peeps' | ||
| # end | ||
|
|
||
| get '/chitter/peeps' do |
There was a problem hiding this comment.
It looks like you tried to organise your routes, however the best way to do this is with a system we call REST. For example with your routes as they are now you would presumably have /chitter/ in front of everything making it unhelpful bloat, and peeps is a bit generic and doesn't give any indication of what that route is for. Perhaps something like get '/peeps' for the landing page and then post '/peeps/new' might be a better solution for your other route. This way we know that these routes are for peeps and that one of them is to create a new one.
| end | ||
|
|
||
| def self.all | ||
| if ENV['ENVIRONMENT'] == 'test' |
There was a problem hiding this comment.
This if else is going to be needed in any function you create and it already adds quite a lot of bloat with just two at the moment, could you extract that into a separate function?
|
Overall this is a nice solution to the problem, well done! I have left some comments where there are a couple things that could be improved, but I also wanted to call out that while having the UI tests is great it would also be good to have unit tests testing your individual functions. If you have any questions about any of the feedback let me know and Im happy to chat it through 😄 |
2 participants
Your name
Please write your full name here to make it easier to find your pull request.
User stories
Please list which user stories you've implemented (delete the ones that don't apply).
README checklist
Does your README contains instructions for
Here is a pill that can help you write a great README!