From 6fd2e83acb218c38aba981fbc4033c2ee7695b0a Mon Sep 17 00:00:00 2001 From: Aayush Sanghavi Date: Sat, 24 Sep 2022 23:54:49 +0530 Subject: [PATCH] review: Add code review comments for TogglTrack Clone --- client/src/App.css | 1 + client/src/App.test.js | 2 + client/src/Components/Homepage/Footer.jsx | 4 +- .../src/Components/Homepage/FooterSignup.jsx | 2 + .../Components/Homepage/secondHome.module.css | 3 + client/src/Components/Loging/Login.jsx | 9 ++- .../responsive Nav/navElements.jsx | 3 + client/src/Components/TimerPage/SubNav.js | 5 ++ client/src/Components/TimerPage/api.js | 3 + client/src/Components/prices/Apps.jsx | 1 + client/src/Components/prices/Faqs.jsx | 4 + .../src/Components/prices/PlanComparison.jsx | 2 + client/src/Components/prices/PricesTop.jsx | 6 ++ client/src/Components/sample.jsx | 2 + client/src/Components/sidebar/Sidebar.jsx | 50 ++++++++++++ client/src/Mainroutes/Navroutes.jsx | 3 + server/backend server/config.js | 12 +++ .../controller/authController.js | 24 +++++- .../controller/taskController.js | 81 ++++++++++++++++++- server/backend server/index.js | 23 +++++- .../middlewares/authentication.js | 4 + server/backend server/models/project.model.js | 5 ++ server/backend server/passport.js | 7 ++ server/backend server/routes/auth.js | 1 + server/backend server/routes/index.js | 2 + server/backend server/routes/project.js | 4 + server/index.js | 2 + 27 files changed, 258 insertions(+), 7 deletions(-) diff --git a/client/src/App.css b/client/src/App.css index eb31e71..0e41bd7 100644 --- a/client/src/App.css +++ b/client/src/App.css @@ -4,3 +4,4 @@ } */ +/* Don't leave empty files in the codebase */ \ No newline at end of file diff --git a/client/src/App.test.js b/client/src/App.test.js index 1f03afe..554ec70 100644 --- a/client/src/App.test.js +++ b/client/src/App.test.js @@ -1,3 +1,5 @@ +// This file should be in a separate tests directory. + import { render, screen } from '@testing-library/react'; import App from './App'; diff --git a/client/src/Components/Homepage/Footer.jsx b/client/src/Components/Homepage/Footer.jsx index 8781e55..402ecb4 100644 --- a/client/src/Components/Homepage/Footer.jsx +++ b/client/src/Components/Homepage/Footer.jsx @@ -61,6 +61,7 @@ export default function Footer() { + {/* The repetition of these tag blocks below could be avoided and replaced with loops. */} TOGGL GLOBAL Blog @@ -112,7 +113,8 @@ export default function Footer() { - + {/* The repetition of tags below could be avoided and replaced with a loop. */} + diff --git a/client/src/Components/Homepage/FooterSignup.jsx b/client/src/Components/Homepage/FooterSignup.jsx index 89c9d08..50ab30f 100644 --- a/client/src/Components/Homepage/FooterSignup.jsx +++ b/client/src/Components/Homepage/FooterSignup.jsx @@ -1,3 +1,5 @@ +// Remove all of these commented lines. + // import { EmailIcon } from '@chakra-ui/icons'; // import { // Flex, diff --git a/client/src/Components/Homepage/secondHome.module.css b/client/src/Components/Homepage/secondHome.module.css index cfb8a3e..efaeb7c 100644 --- a/client/src/Components/Homepage/secondHome.module.css +++ b/client/src/Components/Homepage/secondHome.module.css @@ -1,3 +1,6 @@ +/* Move the jsx and css files for the same component into its own folder +like done for some other components. Follow one convention. */ + /* second div style start from here */ .fixtop{ width: 100%; diff --git a/client/src/Components/Loging/Login.jsx b/client/src/Components/Loging/Login.jsx index 7e85a0d..0c5e5c7 100644 --- a/client/src/Components/Loging/Login.jsx +++ b/client/src/Components/Loging/Login.jsx @@ -58,7 +58,9 @@ export default function Login() { .then((res) => res.json()) .then((res) => { console.log(res) - + // Never string match the error message. Later if the backend changes the error text + // this check will fail and the code will behave unexpectedly. Instead the status code + // or some status enum should be checked for here. if(res.message === "required fields are email,password") { toast({ @@ -113,15 +115,18 @@ export default function Login() { }) - + // This is poor error handling. Logging the error isn't enough. .catch((err) => console.log(err)) } return ( <> + +{/* Please format code. It is all over the place. */} + {/* Such urls should be put in some constants file */} diff --git a/client/src/Components/NavComponents/responsive Nav/navElements.jsx b/client/src/Components/NavComponents/responsive Nav/navElements.jsx index 3262c17..df434a0 100644 --- a/client/src/Components/NavComponents/responsive Nav/navElements.jsx +++ b/client/src/Components/NavComponents/responsive Nav/navElements.jsx @@ -1,3 +1,6 @@ +// Why does this file have a jsx extension when there is no jsx. This prevents the IDE +// from properly highlighting the syntax and showing any suggestions. + import styled from "styled-components"; import { Link } from "react-router-dom"; import {FaBars} from "react-icons/fa"; diff --git a/client/src/Components/TimerPage/SubNav.js b/client/src/Components/TimerPage/SubNav.js index 0cfb292..de2257d 100644 --- a/client/src/Components/TimerPage/SubNav.js +++ b/client/src/Components/TimerPage/SubNav.js @@ -15,6 +15,11 @@ import { import { format } from "date-fns"; import Rangetimer from "./Rangetimer"; +// A common issue that all the components in the codebase suffer from is that the styling is defined inline +// instead of separating that in a dedicated scss or css file. Inline css is usually okay when the component +// is tiny and the css rules are relatively self contained. For such a big component, inline css makes this +// component practically non-reusable, degrades code readbility and clutters the layout and logic contained +// in the component with its styling rules which have little role to play in what the component is about. const SubNav = () => { const[open,setOpen]= useState(false) const [date, setDate] = useState([ diff --git a/client/src/Components/TimerPage/api.js b/client/src/Components/TimerPage/api.js index e936723..e3fc095 100644 --- a/client/src/Components/TimerPage/api.js +++ b/client/src/Components/TimerPage/api.js @@ -1,3 +1,6 @@ +// Don't put utility javascript code along with the components. Have a separate +// utils folder for that. + import axios from 'axios' export const postdata = (send) => { console.log(send) diff --git a/client/src/Components/prices/Apps.jsx b/client/src/Components/prices/Apps.jsx index a8ad43f..5a21b11 100644 --- a/client/src/Components/prices/Apps.jsx +++ b/client/src/Components/prices/Apps.jsx @@ -34,6 +34,7 @@ const Apps = () => { pb="50px" > + {/* Passing styling properties as props is a very strange pattern. Never seen it in well written codebases. */} MOBILE APPS diff --git a/client/src/Components/prices/Faqs.jsx b/client/src/Components/prices/Faqs.jsx index a4e5b55..399ac53 100644 --- a/client/src/Components/prices/Faqs.jsx +++ b/client/src/Components/prices/Faqs.jsx @@ -52,6 +52,10 @@ const Faqs = () => { + {/* Such strings should be declared in one place in a separate class or js file. + For example, see - https://github.com/mozilla/addons-frontend/blob/master/src/amo/constants.js + This makes managing static text easier. It also becomes easier to add support l10n and i18n + so the app can render text for different languages (like rtl languages). */} You will be charged a monthly fee for each member of your team. For paid plans, this fee applies even if you have under 5 active diff --git a/client/src/Components/prices/PlanComparison.jsx b/client/src/Components/prices/PlanComparison.jsx index 2f044e4..ab136ae 100644 --- a/client/src/Components/prices/PlanComparison.jsx +++ b/client/src/Components/prices/PlanComparison.jsx @@ -109,6 +109,8 @@ const PlanComparison = () => { Time Tracking + {/* What is this for? I'm guessing this is for adding some column space. + If so, it is not the right way to do so. Please avoid such hacks. */} diff --git a/client/src/Components/prices/PricesTop.jsx b/client/src/Components/prices/PricesTop.jsx index e7907b9..4d8bba5 100644 --- a/client/src/Components/prices/PricesTop.jsx +++ b/client/src/Components/prices/PricesTop.jsx @@ -149,6 +149,12 @@ const navigate =useNavigate() }} textAlign={"left"} > + {/* The following block could've been written in a much neater fashion by having a + containerwhich assembles each Flex > Box > BsCheckLg unit by looping over a prop + containingdata elements in an array. This is precisely a perfect example of anti-patterns + withReact components. Components are written for being reusable and helping maintain code + better. What is the point of using react and its component framework if y'all hardcode + the entire page like this? Same applies below as well */} diff --git a/client/src/Components/sample.jsx b/client/src/Components/sample.jsx index 67622be..7a4fa44 100644 --- a/client/src/Components/sample.jsx +++ b/client/src/Components/sample.jsx @@ -1,3 +1,5 @@ +// What is this for? Please remove such spurious components which serve no purpose. + import React from 'react' const sample = () => { diff --git a/client/src/Components/sidebar/Sidebar.jsx b/client/src/Components/sidebar/Sidebar.jsx index f133502..041cce3 100644 --- a/client/src/Components/sidebar/Sidebar.jsx +++ b/client/src/Components/sidebar/Sidebar.jsx @@ -1,3 +1,53 @@ +// This file is just unreadable. +// 1. Lots of unused imports +// 2. Lots of commented code +// 3. Absolutely 0 comments on what purpose the sidebar serves +// 4. Tons of CSS mixed along with the component definition and contents +// +// Please write smaller and more well defined components which have a specific purpose. +// Each component should have a few comments about what it does, a sample usage example if possible, etc. +// A good component has a well defined purpose and usually it should be configurable so it can be used elsewhere too. +// From the code below, it looks like Sidebar is a "container" which is a component that is composed of multiple smaller +// independent components. I'm making this distinction because that helps to think about the UI in a different, more +// compartmentalised way that makes organising, writing, reusing and maintaining code easier. +// +// Also, the current way of combining the entire JSX in a single return statement makes understanding the sub-components +// extremely difficult. A better way to use JSX is to define different variables for different logical elements within +// the component. For example (might not be strictly syntactically correct) - +// +// const Header = (props) => ( +//

+// props.message +//

+// ); +// +// const Body = (props) => ( +//
+// props.text +//
+// ); +// +// // ... Similarly Section and Footer are defined. +// +// const Page = (props) => { +// return ( +//
+//
+// +//
+// +// +//
+//
+//
+// ); +// } +// +// The example above makes reading and understanding what the component is doing is much easier. +// +// A good example of a cleanly written big component is - +// https://github.com/mozilla/addons-frontend/tree/master/src/amo/components/AddonsCard. +// It also separates its styling from the component and encapsulates all the points described above. import React,{ReactNode, useEffect, useState} from 'react' import { diff --git a/client/src/Mainroutes/Navroutes.jsx b/client/src/Mainroutes/Navroutes.jsx index c17274c..29fb883 100644 --- a/client/src/Mainroutes/Navroutes.jsx +++ b/client/src/Mainroutes/Navroutes.jsx @@ -1,5 +1,7 @@ import React from 'react' import { Routes,Route } from 'react-router-dom' +// Remove all unused imports from all the files please. Importing something adds an overhead +// while loading the application / component. Avoid this. import Footer from '../Components/Homepage/Footer' import Homepage from '../Components/Homepage/Homepage' import Navbar from "../Components/NavComponents/Navbar" @@ -30,6 +32,7 @@ const Navroutes = () => { } /> } /> } /> + {/* Have consistency in naming your routes. Follow the same convention. */} } /> }/> {/* } /> */} diff --git a/server/backend server/config.js b/server/backend server/config.js index 47a48f7..27d0b11 100644 --- a/server/backend server/config.js +++ b/server/backend server/config.js @@ -1,6 +1,18 @@ const mongoose = require("mongoose"); require("dotenv").config(); +// Typically a database connection pool should be created and used throughout the application. +// Having only a single connection limits the database operations that can be performed at once. +// A connection pool is a resource which manages multiple database connections so that as needed +// the APIs can acquire a connection, use it and return it back to the pool after use. The +// advantage of a pool is that you can perform multiple independent database operations at once, +// improving the performance of your application. Databases can support 100s of concurrent connections +// so using only one connection also leads to wasteage of compute and memory resources of the +// database server. Having only a single connection also means that in case of concurrent API +// invocations only one API call can effectively make progress (the one which is using the database +// connection at the moment). This means that the other API calls will have a much higher wait time +// and thus higher than expected latency. +// Mongoose supports connection pools. See - https://mongoosejs.com/docs/connections.html#connection_pools. const connection = mongoose.connect(process.env.mongo_url); module.exports = { connection, diff --git a/server/backend server/controller/authController.js b/server/backend server/controller/authController.js index 8f8e223..c16a7c1 100644 --- a/server/backend server/controller/authController.js +++ b/server/backend server/controller/authController.js @@ -5,16 +5,33 @@ var jwt = require("jsonwebtoken"); const register = (req, res) => { if ( !req.body.email || !req.body.password) { - return res.send({message:"required fields are email,password"}); + // Explicitly add status codes for error responses. Using a json object for the response body is + // good but not having the correct status code hurts. The statement should be - + // return res.status(400).send({message:"required fields are email,password"}); + // Status code 400 is used for indicating bad requests, where the request was malformed or + // had missing parameters. Also a better way to write the same error message is - + // "Missing required fields - email, password" + return res.send({message:"required fields are email,password"}); } const { email, password } = req.body; bcrypt.hash(password, 8, async (err, hash) => { // Store hash in your password DB. if (err) { - return res.send("error in bcrypt password"); + // This is not a good way to return error responses. APIs should conform to REST principles. + // All error response should be formatted in a certain way so that the client knows what to expect. + // A proper json object with error message and any other relevant information must be sent back + // with an appropriate status code. For internal errors status code 401 should be used. + // Read about status codes here - https://restfulapi.net/http-status-codes/ + // It should be something like - + // res.status(500).send({ error: "Encrypting the password failed" }); + // Also, try to maintain uniformity with responses in the application. Like above and below, both + // error responses are json objects but here it is a string. + return res.send("error in bcrypt password"); } const user = new UserModel({ email, password: hash }); + // What if this operation fails? Always handle for failures and add fallback mechanisms. await user.save(); + // Use status code 201 here. return res.send({message:"Register Successfull"}); }); }; @@ -28,6 +45,9 @@ const login = async (req, res) => { try { const find_user = await UserModel.findOne({ email }); if (!find_user) { + // Avoid words like 'Plz'. Also the real error is that the user was not found. So the + // error message should be something like - "User with email: ${email} not found". + // Should use status code 404. Similarly for all the error cases below. return res.send({message:"Plz enter registered details"}); } diff --git a/server/backend server/controller/taskController.js b/server/backend server/controller/taskController.js index a67b32c..ea2da61 100644 --- a/server/backend server/controller/taskController.js +++ b/server/backend server/controller/taskController.js @@ -1,7 +1,51 @@ const TaskModel = require("../models/Task.model"); +// Each API should have a head comment which provides a good description about: +// 1. what the API does +// 2. what the request looks like +// 3. what responses can be expected (success and failure cases both included) +// post all the data const createTask = async (req, res) => { - try { + // The very first statement in any API function should be to log the incoming request. + // In case of any failure downstream, knowing the incoming request helps a lot during + // debugging and understanding what could've caused the failure. Logging is extremely helpful + // and logging the right things in the right place with the right messages can be the + // difference between spending hours in finding the root error causes vs just looking at the + // logs and figuring it out in 2 minutes. + // + // Each API function should follow the following structure - + // + // const <_http_method_><_resource_> = async (req, res) => { + // console.group('Received request from {req.user.id} to create task, with body: {req.body}.'); + // try { + // console.log('Performing _operation_ with params: {_params_}.') + // // perform API specific operations here + // console.log('Successfully performed _operation_.'); + // res.status(_status_code_) + // } catch (e) { + // console.error('Could not perform _operation_.', e.message); + // // add some retry logic here. + // if (numRetries > retryLimit) { + // res.status(500).send({ "error": "_relevant_error_message_goes_here_" }); + // } + // } + // console.groupEnd(); + // }); + // + + // Deserialisation can be done is a better way. Usually for serialisation and deserialisation + // static functions are defined in the model class itself. Something like this for example - + // + // In models/Task.model.js + // + // // This function deserialises the input json into a Task object. + // static from(task_json, user_json) { + // return Object.assign(new Task(), task_json, user_json); + // } + // + // Its invocation would be something like - + // const task = Task.from(req.body, req.user); + try { const newTask = await TaskModel({ title: req.body.title, id:req.body.id, @@ -14,19 +58,36 @@ const createTask = async (req, res) => { console.log(err); } }; + const getAllTasks = async (req, res) => { try { const allTasks = await TaskModel.find(); // if (!allTasks) return res.send({message:"No Task found"}); return res.send(allTasks); } catch (err) { + // Errors should be logged at the error level, not the default level. console.log(err); + // Error message used below is not clear at all. It gives 0 idea about + // what happened at the server and whether the client should retry the + // request and hope for the correct response the next time. The error + // response, should contain a human readable / understandable message + // with the proper status code. In this case the status code would be + // 500. This statement should be something like - + // res.status(500).send({"error": _stringified_error_}); + // Also, there should be some form of failure handling on the server + // end first, based on the error type. There should be some notion of + // whether the error was transient and would get fixed on retries. + // Error handling in APIs is very crucial for applications working at + // a large scale. res.send("sth went wrong"); } }; const editTasks = async (req,res) => { try{ + // What is the '+' for? It serves no purpose. Avoid such statements. + // Nit: Also req.params.id can be used inline in the 2 places. That + // will avoid creating a new variable. const id= +req.params.id const allTasks = await TaskModel.findOne({id:id}) const update_user = await TaskModel.findOneAndUpdate({id :id}, @@ -44,6 +105,24 @@ const editTasks = async (req,res) => { const deleteTasks = async (req,res) => { try{ const id= +req.params.id + // Always log before and after database (or any IO operations for that matter). + // Certain IO operations might take long to run so it might also be nice to + // read the logs and see how long it takes on average for some operation to + // finish. It will give some idea about what the critical path of the API is. + // This is helpful in figuring out what operations in the API flow should be + // focused on for optimisations. + // + // IO operations also need to have proper error handling. Having a catch-all + // for all kinds of errors is not a good way to do error handling because errors + // from different operations are different and should be treated as such. Some + // errors would be transient like say transaction timeout error or connection + // error but something like duplicate primary key or some other column constraint + // error is not. Knowing the distinction between error types greatly helps in + // making the API code more resilient to failures and improves the success rate + // of the API calls. Large applications with way more complex APIs and a lot more + // moving parts cannot tolerate points of failures. They all have some or the other + // fallback mechanism. Think about such things while writing APIs. Read more + // about how to better handle errors. const data=await TaskModel.findOne({id:id}) const del = await TaskModel.findOneAndDelete({id :id}); res.send({message:"delted",data:del}) diff --git a/server/backend server/index.js b/server/backend server/index.js index 82e49e0..fd1298d 100644 --- a/server/backend server/index.js +++ b/server/backend server/index.js @@ -1,3 +1,7 @@ +// This file is just all over the place. Ideally it should be 10 20 lines at max, +// with 10 lines of imports and 10 lines just initialising the app object. It should +// absolutely not contain any API logic, or routes munging. + const express = require("express"); const { connection } = require("./config"); const cors = require("cors"); @@ -12,6 +16,15 @@ const passport = require('passport'); require('./passport'); +// This is a mess again. All the routes should've been part of routes/index.js. +// Defining half of the stuff there and the other half here is a poor practice. +// Have consistency in your code. Decide and follow one way of doing things +// within the team. This smells like lack of proper team co-ordination. Such +// things should be discussed upfront and sorted out so everyone knows how to +// go about things. Y'all should refer to and follow guidelines laid out by +// the framework authors for best practices. +// In a large project where at least 20 different people write code, if everyone +// decided to do things their way, the code would become unmaintainable very soon. const allRoutes = require("./routes/index"); const authentication = require("./middlewares/authentication"); const ProjectController = require("./routes/project") @@ -28,6 +41,8 @@ app.use(morgan("tiny")); app.use(express.json()); app.use(cookieParser()); app.use("/project", authentication,ProjectController) +// Why is this called timer? Use the name of resource and not some random name for an +// API route. app.use("/timer", authentication,TaskController) // Routes app.use("", allRoutes); @@ -39,6 +54,9 @@ app.get("/", (req, res) => { // oauth +// Delete all the commented code please to improve codebase health. +// Seeing commented code in so many files hurts. + // app.use(cookieSession({ // name: 'google-auth-session', // keys: ['key1', 'key2'] @@ -59,6 +77,9 @@ app.get("/", (req, res) => { // const port = process.env.PORT || 8080 +// All of the APIs below should be in their respective controller and route files. +// This is not the place to add any business logic. + app.get("/login",(req,res)=> { res.send(`Login via google `) }) @@ -114,7 +135,7 @@ app.get("/logout", (req, res) => { }) - +// Don't leave blank lines at random. This is very weird. diff --git a/server/backend server/middlewares/authentication.js b/server/backend server/middlewares/authentication.js index 932ac95..4d5d287 100644 --- a/server/backend server/middlewares/authentication.js +++ b/server/backend server/middlewares/authentication.js @@ -1,4 +1,8 @@ var jwt = require("jsonwebtoken"); + +// Leave a blank line between the require statements and the actual code. +// Also remove all commented log statements. It is better to keep your +// code clean of dead or commented statements. const authentication = (req, res, next) => { // const token = req.headers.authorization; const token = req.cookies.access_token ; diff --git a/server/backend server/models/project.model.js b/server/backend server/models/project.model.js index c253c65..4ddc8ef 100644 --- a/server/backend server/models/project.model.js +++ b/server/backend server/models/project.model.js @@ -1,3 +1,8 @@ +// Leave blank lines where necessary. This looks hotch potchy. +// The naming convention for the model files is also not +// consistent. Make sure to follow one convention (which should +// always be the language standard). Read style guides for the +// right naming practices. const mongoose=require("mongoose") const { Schema } = mongoose; diff --git a/server/backend server/passport.js b/server/backend server/passport.js index baec60b..614a958 100644 --- a/server/backend server/passport.js +++ b/server/backend server/passport.js @@ -1,3 +1,9 @@ +// Why is this file called passport? Because it uses some dependency called passport? +// This is so wrong. Files should be named after the class or functionality they expose. +// That gives the reader some context of what to expect before even opening the file. +// Random names don't help at all. Also, using the same name as another package adds to +// even more confusion. + const passport =require("passport") const GoogleStrategy = require('passport-google-oauth2').Strategy; require("dotenv").config() @@ -35,6 +41,7 @@ passport.deserializeUser(function(user, done) { done(null, user); }); +// Add comments on what this block is about and what it does. passport.use(new GoogleStrategy({ clientID:process.env.CLIENT_ID, clientSecret:process.env.CLIENT_SECRET, diff --git a/server/backend server/routes/auth.js b/server/backend server/routes/auth.js index 6ec90b5..5400d2c 100644 --- a/server/backend server/routes/auth.js +++ b/server/backend server/routes/auth.js @@ -14,6 +14,7 @@ router.get("/hello", (req, res) => { router.post("/register", register); router.post("/login", login); router.get("/logout", logout); +// Either use snake case or camel case. This API path looks off. router.get("/isloggedin", isLoggedIn); module.exports = router; diff --git a/server/backend server/routes/index.js b/server/backend server/routes/index.js index 9447b1e..8258269 100644 --- a/server/backend server/routes/index.js +++ b/server/backend server/routes/index.js @@ -8,6 +8,8 @@ const authentication = require("../middlewares/authentication"); const router = express.Router(); router.use("/auth", authRoutes); +// Resource names in the API path should always be singular. So this should be 'task' +// and the one below should be 'user'. router.use("/tasks", authentication , taskRoutes); router.use("/users", authentication, usersRoutes); diff --git a/server/backend server/routes/project.js b/server/backend server/routes/project.js index d3ace15..aeb8f78 100644 --- a/server/backend server/routes/project.js +++ b/server/backend server/routes/project.js @@ -3,6 +3,10 @@ const express=require("express") const ProjectController= express.Router() const ProjectModel=require("../models/project.model") +// Follow one convention. Define all the API functions in the controllers +// folder and just import that here and define the route using that function. +// Don't define the API function in line with the route. Avoid mixing such +// things. ProjectController.post("/create",async(req,res)=>{ const{id,name,client}=req.body diff --git a/server/index.js b/server/index.js index e69de29..386bb8b 100644 --- a/server/index.js +++ b/server/index.js @@ -0,0 +1,2 @@ +// Why does this empty file exist? Also, there are way too many index.js files +// in the codebase under different sub-directories. \ No newline at end of file