Merge pull request jaegertracing#126 from jaegertracing/style-guide

Style guide
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>

.eslintrc

@@ -17,7 +17,6 @@
     "comma-dangle": 0,
     "no-continue": 0,
     "no-plusplus": 0,
-    "no-restricted-syntax": 0,
     "no-self-compare": 0,
     "no-underscore-dangle": 0,
 

CONTRIBUTING.md

@@ -38,7 +38,7 @@ to be accepted if it:
 By contributing your code, you agree to license your contribution under the terms
 of the [Apache License](LICENSE).
 
-If you are adding a new file it should have a header like below. 
+If you are adding a new file it should have a header like below.
 
 ```
 // Copyright (c) 2017 The Jaeger Authors.
@@ -117,3 +117,17 @@ If you want this to be automatic you can set up some aliases:
 git config --add alias.amend "commit -s --amend"
 git config --add alias.c "commit -s"
 ```
+
+# Style Guide
+
+Prefer to use [flow](https://flow.org/) for new code.
+
+We use [`prettier`](https://prettier.io/), an "opinionated" code formatter. It
+can be applied to both JavaScript and CSS source files via `yarn prettier`.
+
+Then, most issues will be caught by the linter, which can be applied via `yarn
+eslint`.
+
+Finally, we generally adhere to the
+[Airbnb Style Guide](https://github.com/airbnb/javascript), with exceptions as
+noted in our `.eslintrc`.

package.json

@@ -81,12 +81,20 @@
     "lint": "npm run eslint && npm run prettier && npm run flow && npm run check-license",
     "eslint": "eslint src",
     "check-license": "./scripts/check-license.sh",
-    "prettier": "prettier --single-quote --trailing-comma es5 --print-width 110 --write \"src/**/*.js\"",
+    "prettier": "prettier --write \"src/**/*.js\" \"src/**/*.css\"",
     "flow": "flow; test $? -eq 0 -o $? -eq 2",
     "precommit": "lint-staged"
   },
   "jest": {
-    "collectCoverageFrom": ["src/**/*.js", "!src/utils/DraggableManager/demo/*.js"]
+    "collectCoverageFrom": [
+      "src/**/*.js",
+      "!src/utils/DraggableManager/demo/*.js"
+    ]
+  },
+  "prettier": {
+    "printWidth": 110,
+    "singleQuote": true,
+    "trailingComma": "es5"
   },
   "lint-staged": {
     "*.js": [

src/actions/jaeger-api.test.js

@@ -106,7 +106,10 @@ it('@JAEGER_API/FETCH_SERVICES should return a promise', () => {
 it('@JAEGER_API/FETCH_SERVICE_OPERATIONS should call the JaegerAPI', () => {
   const api = JaegerAPI;
   const mock = sinon.mock(api);
-  const called = mock.expects('fetchServiceOperations').once().withExactArgs('service');
+  const called = mock
+    .expects('fetchServiceOperations')
+    .once()
+    .withExactArgs('service');
   jaegerApiActions.fetchServiceOperations('service');
   expect(called.verify()).toBeTruthy();
   mock.restore();

src/api/jaeger.js

@@ -32,11 +32,9 @@ function getJSON(url, query) {
         .then(({ errors = [] }) => {
           throw new Error(errors.length > 0 ? errors[0].msg : 'An unknown error occurred.');
         })
-        .catch(
-          (/* err */) => {
-            throw new Error('Bad JSON returned from the Jaeger Query Service.');
-          }
-        );
+        .catch((/* err */) => {
+          throw new Error('Bad JSON returned from the Jaeger Query Service.');
+        });
     }
     return response.json();
   });

src/components/App/App.css

@@ -34,20 +34,20 @@ body ::-webkit-scrollbar {
 }
 
 a {
-  color: #11939A;
+  color: #11939a;
 }
 
 a:hover {
-  color: #00474E;
+  color: #00474e;
   cursor: pointer;
 }
 
 .clearfix:after {
-   content: " ";
-   visibility: hidden;
-   display: block;
-   height: 0;
-   clear: both;
+  content: ' ';
+  visibility: hidden;
+  display: block;
+  height: 0;
+  clear: both;
 }
 
 .pull-left {
@@ -83,19 +83,21 @@ a:hover {
 }
 
 .ui.table td.light-grey {
-  background-color: #F1F1F1;
+  background-color: #f1f1f1;
 }
 
-.ui.table, .ui.table thead tr:first-child>th:last-child, .ui.table thead tr:first-child>th:first-child {
+.ui.table,
+.ui.table thead tr:first-child > th:last-child,
+.ui.table thead tr:first-child > th:first-child {
   border-radius: 0;
 }
 
 .ui.table thead th {
-  background-color: #F1F1F1;
+  background-color: #f1f1f1;
 }
 
 .ui.sortable.table thead th.sorted,
 .ui.sortable.table thead th:hover,
 .ui.sortable.table thead th.sorted:hover {
-  background-color: #D6D6D5;
+  background-color: #d6d6d5;
 }

src/components/App/NotFound.js

@@ -1,3 +1,5 @@
+// @flow
+
 // Copyright (c) 2017 Uber Technologies, Inc.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -12,40 +14,32 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import PropTypes from 'prop-types';
 import React from 'react';
 import { Link } from 'react-router-dom';
 
 import prefixUrl from '../../utils/prefix-url';
 
-export default function NotFound({ error }) {
+type NotFoundProps = {
+  error: any,
+};
+
+export default function NotFound({ error }: NotFoundProps) {
   return (
     <section className="ui container">
       <div className="ui center aligned basic segment">
         <div className="ui center aligned basic segment">
-          <h1>
-            {'404'}
-          </h1>
-          <p>
-            {"Looks like you tried to access something that doesn't exist."}
-          </p>
+          <h1>{'404'}</h1>
+          <p>{"Looks like you tried to access something that doesn't exist."}</p>
         </div>
-        {error &&
+        {error && (
           <div className="ui red message">
-            <p>
-              {String(error)}
-            </p>
-          </div>}
+            <p>{String(error)}</p>
+          </div>
+        )}
         <div className="ui center aligned basic segment">
-          <Link to={prefixUrl('/')}>
-            {'Back home'}
-          </Link>
+          <Link to={prefixUrl('/')}>{'Back home'}</Link>
         </div>
       </div>
     </section>
   );
 }
-
-NotFound.propTypes = {
-  error: PropTypes.object,
-};

src/components/App/Page.js

@@ -55,9 +55,7 @@ class Page extends React.Component<PageProps> {
       <section className="jaeger-ui-page" id="jaeger-ui">
         <Helmet title="Jaeger UI" />
         <TopNav menuConfig={menu} />
-        <div className="jaeger-ui--content">
-          {children}
-        </div>
+        <div className="jaeger-ui--content">{children}</div>
       </section>
     );
   }

src/components/DependencyGraph/DAG.js

@@ -21,17 +21,20 @@ import dagre from 'dagre';
 cydagre(cytoscape, dagre);
 
 export default class DAG extends React.Component {
-  static get propTypes() {
-    return {
-      serviceCalls: PropTypes.arrayOf(
-        PropTypes.shape({
-          parent: PropTypes.string,
-          child: PropTypes.string,
-          callCount: PropTypes.number,
-        })
-      ),
-    };
-  }
+  static propTypes = {
+    serviceCalls: PropTypes.arrayOf(
+      PropTypes.shape({
+        parent: PropTypes.string,
+        child: PropTypes.string,
+        callCount: PropTypes.number,
+      })
+    ),
+  };
+
+  static defaultProps = {
+    serviceCalls: [],
+  };
+
   componentDidMount() {
     const { serviceCalls } = this.props;
     const nodeMap = {};

src/components/DependencyGraph/DependencyForceGraph.js

@@ -65,9 +65,11 @@ export default class DependencyForceGraph extends Component {
 
     return (
       <div
-        ref={/* istanbul ignore next */ c => {
-          this.container = c;
-        }}
+        ref={
+          /* istanbul ignore next */ c => {
+            this.container = c;
+          }
+        }
         style={{ position: 'relative' }}
       >
         <InteractiveForceGraph
@@ -91,7 +93,7 @@ export default class DependencyForceGraph extends Component {
           nodeAttrs={['orphan']}
           highlightDependencies
         >
-          {nodes.map(({ labelStyle, labelClass, showLabel, opacity, fill, ...node }) =>
+          {nodes.map(({ labelStyle, labelClass, showLabel, opacity, fill, ...node }) => (
             <ForceGraphNode
               key={node.id}
               node={node}
@@ -101,10 +103,10 @@ export default class DependencyForceGraph extends Component {
               opacity={opacity}
               fill={fill}
             />
-          )}
-          {links.map(({ opacity, ...link }) =>
+          ))}
+          {links.map(({ opacity, ...link }) => (
             <ForceGraphLink key={`${link.source}=>${link.target}`} opacity={opacity} link={link} />
-          )}
+          ))}
         </InteractiveForceGraph>
       </div>
     );

src/components/DependencyGraph/DependencyGraph.css

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 .rv-force__node {
-  fill: #11939A;
+  fill: #11939a;
   cursor: pointer;
 }
 

src/components/DependencyGraph/index.js

@@ -28,16 +28,22 @@ import DependencyForceGraph from './DependencyForceGraph';
 import DAG from './DAG';
 
 export default class DependencyGraphPage extends Component {
-  static get propTypes() {
-    return {
-      dependencies: PropTypes.any,
-      fetchDependencies: PropTypes.func.isRequired,
-      nodes: nodesPropTypes,
-      links: linksPropTypes,
-      loading: PropTypes.bool,
-      error: PropTypes.object,
-    };
-  }
+  static propTypes = {
+    // eslint-disable-next-line react/forbid-prop-types
+    dependencies: PropTypes.any.isRequired,
+    fetchDependencies: PropTypes.func.isRequired,
+    nodes: nodesPropTypes,
+    links: linksPropTypes,
+    loading: PropTypes.bool.isRequired,
+    // eslint-disable-next-line react/forbid-prop-types
+    error: PropTypes.object,
+  };
+
+  static defaultProps = {
+    nodes: null,
+    links: null,
+    error: null,
+  };
 
   constructor(props) {
     super(props);
@@ -85,14 +91,14 @@ export default class DependencyGraphPage extends Component {
     return (
       <div className="my2">
         <Menu tabular>
-          {GRAPH_TYPE_OPTIONS.map(option =>
+          {GRAPH_TYPE_OPTIONS.map(option => (
             <Menu.Item
               active={graphType === option.type}
               key={option.type}
               name={option.name}
               onClick={() => this.handleGraphTypeChange(option.type)}
             />
-          )}
+          ))}
         </Menu>
         <div
           style={{

src/components/SearchTracePage/SearchDropdownInput.js

@@ -65,6 +65,7 @@ export default class SearchDropdownInput extends Component {
 
 SearchDropdownInput.defaultProps = {
   maxResults: 250,
+  items: [],
 };
 SearchDropdownInput.propTypes = {
   items: PropTypes.arrayOf(
@@ -76,6 +77,6 @@ SearchDropdownInput.propTypes = {
   input: PropTypes.shape({
     value: PropTypes.string,
     onChange: PropTypes.func,
-  }),
+  }).isRequired,
   maxResults: PropTypes.number,
 };

src/components/SearchTracePage/TraceResultsScatterPlot.js

@@ -45,12 +45,11 @@ function TraceResultsScatterPlotBase(props) {
           onValueMouseOut={onValueOut}
           data={data}
         />
-        {overValue &&
+        {overValue && (
           <Hint value={overValue}>
-            <h4 className="scatter-plot-hint">
-              {overValue.name || '¯\\_(ツ)_/¯'}
-            </h4>
-          </Hint>}
+            <h4 className="scatter-plot-hint">{overValue.name || '¯\\_(ツ)_/¯'}</h4>
+          </Hint>
+        )}
       </XYPlot>
     </div>
   );