Skip to content

Instantly share code, notes, and snippets.

@amrvitale
Created July 23, 2020 00:29
Show Gist options
  • Save amrvitale/1dab162cea88872c56e280756539a7fc to your computer and use it in GitHub Desktop.
Save amrvitale/1dab162cea88872c56e280756539a7fc to your computer and use it in GitHub Desktop.
capstone feedback
Hi Angela,
I'm Jeff from the Grading Team. Good job on your assignment so far.
Please review the following grading criteria and fix any items that I've marked with an ❌ .
Good job on all the items that I've marked with a πŸ‘ !
Feedback Key
[ πŸ‘ ] = What you did well
[ ❌ ] = Required changes to pass assignment
Requirements
[ πŸ‘ ] This app does something interesting or useful (i.e. there are users for this app)
[ πŸ‘ ] This app uses the following stack: React, Node, Express, PostgreSQL (students may only use a different stack if first approved by their Program Manager)
[ ❌ ] Both client- and server-side code must be tested.
[ ❌ ] I ran your app's tests and noticed that some of them do not pass. For example the test src/ctf/CTF.test.js failed. Please fix any failing tests (and any skipped tests) and resubmit your app.
In a professional development environment a continuous integration server will automatically run your tests whenever you submit a merge request, and reject your submission automatically if any of your tests fail. Some environments are even configured to automatically run your tests every time you git commit locally. Passing tests indicate an attention to detail to potential employers.
[ ❌ ] You are required to test the happy path for each endpoint in the API. For example, I was expecting to find an API test for this endpoint/route but was unable to do so. Please add the missing tests and resubmit your app.
[ ❌ ] This app is responsive and works just as well on mobile devices as it does on desktop devices.
[ ❌ ] At some large screen resolutions your layout does not make sense visually. In the example below I'd expect to see the entire view title (My Draftlings Desk) within the viewport.
screenshot
[ ❌ ] At this large screen resolution this view's header is unnecessarily large and pushes the useful content (the Story tiles) down such that scrolling is necessary. Please reduce the height of this header so that your app does not waste this viewport space. screenshot
[ ❌ ] Currently you are not using mobile-first CSS, as indicated by your use of a max-width @media query in your CSS. To write mobile-first CSS, design your CSS for a mobile screen resolution and put those styles at the top of your CSS file outside of any media queries. Then override any styles that need to change with a larger screen by putting those styles in a min-width @media query below your default, mobile screen queries.The goal is to write styles for the smallest screens first and then add styling for progressively larger screens as necessary. Your first media breakpoint should be for tablet sized screens using a min-width @media query. See this example below:
body {
/*default styles here,
optimized for mobile screens */
}
@media screen and (min-width:480px) {
body {
/* add/override styles as necessary for wider (480px+) screens here */
}
}
@media screen and (min-width:800px) {
body {
/* further add/override styling for even wider (800px+) screens here */
}
}
[ πŸ‘ ] The content on the app is clear and readable.
[ πŸ‘ ] The students did not use styling frameworks like Bootstrap (they should only be using vanilla CSS).
[ ❌ ] Both GitHub repos include comprehensive README files, which include:
The name of the app at the top of the file
A link to the live project
Documentation of the API.
Screenshot(s) of the app.
A summary section. This should have a concise explanation of what the app does.
A section on the technology
[ ❌ ] Your API readme.md is missing all of the above information.
[ πŸ‘ ] The app includes a landing page that explains what the app does and how to get started, in addition to pages required to deliver the main functionality.
[ πŸ‘ ] There is a live, publicly-accessible version of the app at a custom URL.
[ ❌ ] The app includes a Favicon.
[ ❌ ] Please do not use the default React App favicon. Consider making your own favicon here.
[ πŸ‘ ] The app works across different browsers.
N/A: If the app requires authentication, it includes demo credentials.
Styling
Please rate the app's appearance 0-4 based on the following (they must score a 3 to pass):
[ ❌ ] 2 - The student has styled their app, but it doesn't follow basic best practices around font, layout, and color. Or the student styled their app with a framework like Bootstrap.
[ ❌ ] Please see my layout concerns mentioned above (max-width and large screen layout issues).
Code
The student earns 1 point for each of the following and must earn a total of 3 points to pass:
[ ❌ ] Their code follows consistent standards (use of indentation, semicolons, etc)
[ ❌ ] I noticed that you use semicolons inconsistently, even within a given source code file. We recommend the AirBnB JavaScript coding standard, which requires the use of semicolons. Semicolons unambiguously terminate a JavaScript statement and consequently reduce bugs. Consider using the jshint or ES Lint extensions for Visual Studio Code to find issues with your current coding style (including missing semicolons). Follow its recommendations to improve your style and potentially reduce bugs.
[ πŸ‘ ] There are no errors in the console when the app is running
[ πŸ‘ ] Their code follows sound architectural patterns (the API is RESTful, separation of concerns, semantic HTML/JSX etc)
[ ❌ ] The student has included comments where appropriate
[ ❌ ] I wasn't able to find any descriptive comments in your app code, which is a requirement for this assignment. Consider describing what your React components do in a comment above the component declaration. Consider writing your comments in the JSDoc format so that code editors can parse and present them to you via IntelliSense. See this Visual Studio Code documentation for an example of how that feature works.
Additional Required Revisions
[ ❌ ] Your sample data is a bit confusing and not portfolio-ready. Please remove the duplicate titles and "Testing..." text. Consider retitling the story named "Panic Button". At first I thought that was supposed to literally be an interactive button in your app. screenshot
Optional Feedback
Consider making the HTML title of you app the name of your app.
It looks like you've got a good start on this assignment; keep up the good work!
If anything here that I’ve mentioned is unclear, please don’t hesitate to reach out for technical assistance via Slack: https://www.bloc.io/resources/getting-unstuck. If it’s a question about the feedback, feel free to resubmit with a question.
Want to learn more? Check out our group sessions & QA resources page http://bit.ly/gs-g-home with hours of recorded video and live sessions.
Thanks,
Jeff Buda
Grading Team
Graded by Jeff Buda.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment