Skip to content

Commit

Permalink
Merge pull request #126 from jaegertracing/style-guide
Browse files Browse the repository at this point in the history
Style guide
tiffon authored Nov 28, 2017

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents 826dfda + b8fa824 commit 85f9822
Showing 65 changed files with 1,128 additions and 1,073 deletions.
1 change: 0 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -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,

16 changes: 15 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -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`.
12 changes: 10 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -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": [
5 changes: 4 additions & 1 deletion src/actions/jaeger-api.test.js
Original file line number Diff line number Diff line change
@@ -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();
8 changes: 3 additions & 5 deletions src/api/jaeger.js
Original file line number Diff line number Diff line change
@@ -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();
});
24 changes: 13 additions & 11 deletions src/components/App/App.css
Original file line number Diff line number Diff line change
@@ -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;
}
34 changes: 14 additions & 20 deletions src/components/App/NotFound.js
Original file line number Diff line number Diff line change
@@ -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,
};
4 changes: 1 addition & 3 deletions src/components/App/Page.js
Original file line number Diff line number Diff line change
@@ -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>
);
}
25 changes: 14 additions & 11 deletions src/components/DependencyGraph/DAG.js
Original file line number Diff line number Diff line change
@@ -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 = {};
16 changes: 9 additions & 7 deletions src/components/DependencyGraph/DependencyForceGraph.js
Original file line number Diff line number Diff line change
@@ -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>
);
2 changes: 1 addition & 1 deletion src/components/DependencyGraph/DependencyGraph.css
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@ limitations under the License.
*/

.rv-force__node {
fill: #11939A;
fill: #11939a;
cursor: pointer;
}

30 changes: 18 additions & 12 deletions src/components/DependencyGraph/index.js
Original file line number Diff line number Diff line change
@@ -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={{
3 changes: 2 additions & 1 deletion src/components/SearchTracePage/SearchDropdownInput.js
Original file line number Diff line number Diff line change
@@ -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,
};
9 changes: 4 additions & 5 deletions src/components/SearchTracePage/TraceResultsScatterPlot.js
Original file line number Diff line number Diff line change
@@ -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>
);
Loading

0 comments on commit 85f9822

Please sign in to comment.