Merge pull request #78 from cornell-dti/add-coding-guide

Updated README and added style guide

README.md

@@ -1,5 +1,7 @@
 # Project Samwise v1.0
 
+_Last updated **02/04/2019**_.
+
 ## Contents
 
 - [About](#about)
@@ -15,50 +17,87 @@ A web application designed to help Cornell students plan their semesters.
 ​
 ## Getting Started
 
-You will need the latest npm or yarn to run the frontend and at least python 3.6 to run the backend.
+### Backend
 
-To run the frontend, go to the frontend folder and run `npm run start` or `yarn start`. To build the
-frontend, run `npm run build` or `yarn build`.
+You will need the latest npm or yarn to run the frontend and at least python 3.7 to run the backend.
+Although previous versions of Python may successfully run the code, we want the python version to
+be consistent with the python version on the server.
 
-Before running the backend, you also need to setup some environment variables. The format is shown
-below
+The following operations needs to be performed in the `backend/` folder to run the backend locally.
 
+1. You need to have `pip3` installed and use `pip3 install -r requirements.txt` to install all the
+   backend dependencies. When some dependencies changes, you have to run this again.
+2. You need to setup all the environment variables in the `backend/.env` file. These information 
+   will be auto-loaded in the python code. The format of the environment variable file is shown 
+   below.
+
+   You need to ask a team member about the the value of the secret `DATABASE_URL` and 
+   `FIREBASE_CONFIG`. If you are not a team member, you can get those on your own in some cloud 
+   services.
+
 ```bash
-# In file backend/.env
 export DATABASE_URL=the-secret-postgres-url-for-development
-FIREBASE_CONFIG=app/serviceAccount.json
+FIREBASE_SERVICE_CONFIG="THE_SECRET_FIREBASE_SERVICE_CONFIG"
 DEBUG=true
 ```
 
-You need to ask a team member about the the value of the secret `DATABASE_URL` and 
-`FIREBASE_CONFIG`. If you are not a team member, you can get those on your own in some cloud 
-services.
+After you performed the above mentioned operations, you can run the bash script `./start_server`
+or just `python3 run.py` to start the server.
 
-_Last updated **02/04/2019**_.
+### Frontend
+
+To run the frontend, go to the frontend folder and run `yarn start` or `npm run start`. To build the
+frontend, run `yarn build` or `npm run build`. Although Yarn and NPM are both supported, we
+recommend Yarn because it's much faster and deterministic.
+
+We use various tools to ensure the quality of our frontend code. They should be installed and
+properly configured with your IDE or text editor.
+
+### Contribution
+
+We disabled everyone's ability to directly commit to master branch to ensure code quality. To make
+changes to the code, you can create a new branch, have some changes in your branch, and create it
+a pull request with good changelog.
+
+You are expected to follow the [Style Guide](docs/style-guide.md) in your contributed code.
 
 ## Dependencies & Libraries
- * [React (16.8.1)](https://reactjs.org/) - a Facebook library for frontend. We use it for frontend UI.
- * [Redux (4.0.1)](https://redux.js.org/) - a predictable state container for JavaScript apps. We used it for state management.
- * [React-Redux (5.1.1)](https://github.com/reduxjs/react-redux) - a library for bindings between React and Redux.
- * [Firebase (5.8.0)](https://firebase.google.com) - a serverless service for auth, database, etc. We used it for auth.
- * [React FirebaseUI (3.1.2)](https://github.com/firebase/firebaseui-web-react) - a simple sign-in component library for firebase auth.
- * [Semantic UI React (0.85.0)](https://react.semantic-ui.com/) - a library of pre-styled components in Semantic UI for React.
- * [React Calendar (2.14.1)](https://www.npmjs.com/package/react-calendar) - A calendar component for React.
- * [React Color (2.14.1)](https://casesandberg.github.io/react-color/) - a collection of React components for picking colors.
- * [React Search Box (2.0.1)](https://ghoshnirmalya.github.io/react-search-box/) - a search box component for React.
- * [React Toastify (4.5.0)](https://fkhadra.github.io/react-toastify/) - a library for emitting toasts in React.
-​
+
+### Frontend
+
+* [React](https://reactjs.org/) - a Facebook library for frontend. We use it for frontend UI.
+* [Redux](https://redux.js.org/) - a predictable state container for JavaScript apps. We used it for state management.
+* [React-Redux)](https://github.com/reduxjs/react-redux) - a library for bindings between React and Redux.
+* [Firebase](https://firebase.google.com) - a serverless service for auth, database, etc. We used it for auth.
+* [React FirebaseUI](https://github.com/firebase/firebaseui-web-react) - a simple sign-in component library for firebase auth.
+* [Semantic UI React](https://react.semantic-ui.com/) - a library of pre-styled components in Semantic UI for React.
+* [React Calendar)](https://www.npmjs.com/package/react-calendar) - A calendar component for React.
+* [React Color)](https://casesandberg.github.io/react-color/) - a collection of React components for picking colors.
+* [React Search Box](https://ghoshnirmalya.github.io/react-search-box/) - a search box component for React.
+* [React Toastify](https://fkhadra.github.io/react-toastify/) - a library for emitting toasts in React.
+
+To know about the specific versions of these dependencies, go to [package.json](frontend/package.json).
+
+## Backend
+
+* [Firebase](https://firebase.google.com) - a serverless service for auth, database, etc. We used it for auth.
+* [Flask](http://flask.pocoo.org/) - a Python backend framework.
+* [SQLAlchemy](https://www.sqlalchemy.org/) - a Python database toolkit.
+* [Flask-SQLAlchemy](http://flask-sqlalchemy.pocoo.org/2.3/) - a Flask extension for SQLAlchemy.
+
+To know about the specific versions of these dependencies, go to [package.json](backend/requirements.txt).
+
 ## External Documentation
 
-* [Backend API Documentation](https://apiary.io/) - this is an external Apiary documenting the endpoints for our application.
-​
+* [Backend API Documentation](https://samwise.docs.apiary.io/) - this is an external Apiary documenting the endpoints for our application.
+* [Design Docs](docs/design-docs.md) - this is a place where we document some of our design decisions.
+
 ## Screenshots
 
 <img src="./screenshots/placeholder1.png" width="250px" style="margin: 10px; border: 1px rgba(0,0,0,0.4) solid;">
-​
 
 ## Contributors
-​
+
 **Since 2016**
 * **Alice Zhou** - Product Manager
 * **Justin Tran** - Front-end Developer
@@ -74,8 +113,6 @@ _Last updated **02/04/2019**_.
 * **Michael Xing** - Front-end Developer
 * **Michelle Park** - Designer
 * **Sam Zhou** - Front-end Developer
-​
 
 We are a team within **Cornell Design & Tech Initiative**. For more information, see our website [here](https://cornelldti.org/).
 <img src="https://raw.githubusercontent.com/cornell-dti/design/master/Branding/Wordmark/Dark%20Text/Transparent/Wordmark-Dark%20Text-Transparent%403x.png">
-​

docs/design-docs.md

@@ -0,0 +1,3 @@
+# Design Document
+
+Some of the important design decisions should be documented here.

docs/flow-guide.md

@@ -0,0 +1,115 @@
+# Flow Style Guide
+
+Flow is Facebook's static style checker for JavaScript. While this tool enables you to write
+idiomatic JavaScript code, you should follow these specific guidelines to ensure a better code
+quality.
+
+The rules are listed below with examples and justifications.
+
+Note that this is only a guideline, or a tutorial. You should learn flow on its
+[official website](https://flow.org/en/docs/).
+
+## Always Type Annotated React Components
+
+React components can become very complex over time. There may be 
+[ten different things](../frontend/src/components/Util/TaskEditors/TaskEditor.jsx) you need to pass
+into a component. With type declarations, the users of those component can easily check what needs
+to be passed. Also, when it's the time for refactoring, type checker can ensure that you fixed all
+the references and usages of the component to be refactored.
+
+## Favor Immutability Over Mutability
+
+Idiomatic react code generally favors the immutable and functional style. Here are some examples: 
+
+- `this.state = [something]` does not work. You have to use `this.setState()`.
+- The `render()` function must be a pure function of `this.props` and `this.state`.
+- The content redux store should never be mutated manually, but it should be updated by reducers.
+- React's pure component only shallowly checks the content, so only immutable data structure works.
+- ...
+
+Since JavaScript is a dynamic language, immutability cannot be enforced. Therefore, we must have
+tools like Flow or TypeScript to enforce it for us during development.
+
+In Flow, you should write
+
+```javascript
+type State = {|
+    +name: string;
+    +age: number;
+|};
+```
+
+instead of
+
+```javascript
+type State = {|
+    name: string;
+    age: number;
+|};
+```
+
+Note that the `+` sign indicates that the field is immutable. You can learn more about the object
+type declaration [here](https://flow.org/en/docs/types/objects/). 
+
+Immutable declarations also enabled flow to have better 
+[type refinement](https://flow.org/en/docs/lang/refinements/).
+
+Consider the code below:
+
+```javascript
+type Person = {|
+    +name: string;
+    bestFriend: Person | null;
+|}
+
+function someOtherFunction(person: Person) {}
+
+function printName(name: string) {}
+
+function test(person: Person) {
+    if (person.bestFriend != null) {
+        someOtherFunction(person);
+        printName(person.bestFriend.name); // Flow yells: person.bestFriend may be null
+    }
+}
+```
+
+Flow thinks that `person.bestFriend` will be `null` even if we checked it above. This behavior is
+correct because `someOtherFunction` may set it to `null` again since it's not immutable. If the
+field `bestFriend` is declared as immutable.
+
+## Favor Exact Object Type Over Inexact Object Type
+
+Flow supports both exact and inexact object types. However, exact object types generally ensure
+better and cleaner code and the Flow team planned to 
+[have exact object type by default](https://medium.com/flow-type/on-the-roadmap-exact-objects-by-default-16b72933c5cf)
+in the future.
+
+Their difference can be explained in the code below.
+
+```javascript
+type ExactPerson = {| +name: string |};
+type InexactPerson = { +name: string };
+type DeadPerson = { +name: string; +lastWords: string };
+
+function acceptExactPerson(person: ExactPerson) {}
+function acceptInexactPerson(person: InexactPerson) {}
+
+function test() {
+    acceptExactPerson({ name: 'Alice' }); // Passed Check
+    acceptExactPerson({ name: 'Bob', lastWords: 'Ahh' }); // Failed Check. Flow: Extra field lastWords.
+    acceptInexactPerson({ name: 'Alice' }); // Passed Check
+    acceptInexactPerson({ name: 'Hacked User', intro: 'Ahh' }); // Also passed check, but do you want this?
+}
+```
+
+You can see that inexact object type allows everything as long as it has all the required fields.
+Although it's more flexible, it may cover up some potential problems. For example, you may add
+another field `intro` to the type `InexactPerson`, and you want Flow to give you compile error so 
+you know where to refactor your code. However, there may be already some places where you do passed
+in the field `intro`, but they may mean different things. The type checker cannot find this problem
+because it passes both the original test and the current test.
+
+In the context of React component, you generally want your component to do only one thing and do
+it in a modular way. Therefore, you really don't want to accidentally pass in extra fields because
+some changes elsewhere may break the modular encapsulation in the future in unexpected ways. 

docs/style-guide.md

@@ -0,0 +1,87 @@
+# Style Guide
+
+## General Rules
+
+The repo has already been setup with some linter config and style guides. They should be 
+automatically installed when you are installing the dependencies.
+
+You should configure your IDE so that it will warn you when it's violated. You should not ignore
+those linter warnings. You are expected to follow the guides given by the linters and only commit 
+code that passes the linter checks.
+
+When you have doubts about why certain linting rules exist, you should first search online to
+understand it's rationale. If you are unsure about how to make the linter happy, please ask another
+team member.
+
+However, sometimes you may need to suppress some linter checks. In those circumstances, you should
+document the reason why you must suppress these checks. It should only be occasionally used.
+
+## Backend
+
+Always follow the [PEP 8](https://www.python.org/dev/peps/pep-0008/) style guide.
+
+IDE Integration:
+
+- PyCharm: PEP 8 linting enabled by default.
+- VSCode: [link](https://code.visualstudio.com/docs/python/linting)
+- Atom: [link](https://atom.io/packages/pep8)
+
+## Frontend
+
+### ESLint
+
+Always follow the [airbnb JavaScript style guide](https://github.com/airbnb/javascript). We already
+have the [ESLint](https://eslint.org) [config](../frontend/.eslintrc) setup in the repo.
+
+ESLint IDE Integration:
+
+- JetBrains family: [IntelliJ](https://www.jetbrains.com/help/idea/eslint.html),
+  [WebStorm](https://www.jetbrains.com/help/webstorm/eslint.html)
+- VSCode: [link](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint)
+- Atom: [link](https://atom.io/packages/linter-eslint)
+- Others: [ESLint Integration Page](https://eslint.org/docs/user-guide/integrations)
+
+### Flow
+
+Besides ESLint, we also use [Flow](https://flow.org) to statically type-check our frontend code. 
+Since JavaScript is a dynamic language with many 
+[surprises](https://charlieharvey.org.uk/page/javascript_the_weird_parts) and our frontend code is
+inherently complex, this is a must. You are expected to add type annotations to your React
+components and pass the Flow static type checker. Here is [our Flow guideline](flow-guide.md) for 
+you to follow.
+
+Flow should be automatically installed in `node_modules/` when you are installing frontend
+dependencies. You can follow the instructions on [Flow's website](https://flow.org/en/docs/install/)
+if you encountered any problems. The config of the flow is listed [here](../frontend/.flowconfig).
+
+To run flow locally to type-check your code, you can run `yarn run flow` or `flow` if you have
+installed flow globally. Flow can provide reliable type-based auto-completion service for your IDEs,
+so you should properly configure it.
+
+Flow IDE Integration:
+
+- JetBrains family: [IntelliJ](https://www.jetbrains.com/help/idea/using-the-flow-type-checker.html),
+  [WebStorm](https://www.jetbrains.com/help/webstorm/using-the-flow-type-checker.html)
+- VSCode: [Flow Language Support](https://marketplace.visualstudio.com/items?itemName=flowtype.flow-for-vscode) 
+  or [vscode-flow-ide](https://marketplace.visualstudio.com/items?itemName=gcazaciuc.vscode-flow-ide)
+- Atom: [Flow for Atom IDE](https://atom.io/packages/ide-flowtype)
+- Others: [Flow Editors Page](https://flow.org/en/docs/editors/)
+
+Flow also provides some community-maintained library definitions. You should install them so that
+you can have better auto-completion when using those dependencies. 
+
+You should install the [flow-typed](https://github.com/flow-typed/flow-typed) tool globally to
+manage all the type declaration we will use. When we update the version of certain dependencies,
+you should run `flow-typed` again.
+
+## Documentation
+
+You should document all your code. 
+
+For each backend endpoints, the behavior and the interface should be documented on our 
+[Apiary page](https://samwise.docs.apiary.io/).
+
+For each frontend components and functions, you should give sufficient information to enable the
+user of the component/function to completely understand what needs to be passed in. If the type
+signature and the component/function name is always obvious what's going on, you **don't** need to
+repeat it in the docs.

frontend/.flowconfig

@@ -5,7 +5,6 @@
 [libs]
 
 [lints]
-all=error
 deprecated-utility=warn
 [options]
 esproposal.class_instance_fields=enable