Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style guide #126

Merged
merged 4 commits into from
Nov 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
16 changes: 15 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Up @@ -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": [
Expand Down
5 changes: 4 additions & 1 deletion src/actions/jaeger-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 3 additions & 5 deletions src/api/jaeger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
24 changes: 13 additions & 11 deletions src/components/App/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand Up @@ -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>
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/components/App/TopNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ export default function TopNav(props: TopNavProps) {
<div className="ui input">
<TraceIDSearchInput />
</div>
{NAV_LINKS.map(({ key, to, text }) =>
{NAV_LINKS.map(({ key, to, text }) => (
<Link key={key} to={to} className="item">
{text}
</Link>
)}
))}
<div className="right menu">
{menuItems.map(item => {
if (item.items) {
Expand Down
25 changes: 14 additions & 11 deletions src/components/DependencyGraph/DAG.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down
16 changes: 9 additions & 7 deletions src/components/DependencyGraph/DependencyForceGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}
Expand All @@ -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>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/DependencyGraph/DependencyGraph.css
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ limitations under the License.
*/

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

Expand Down
30 changes: 18 additions & 12 deletions src/components/DependencyGraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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={{
Expand Down
3 changes: 2 additions & 1 deletion src/components/SearchTracePage/SearchDropdownInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export default class SearchDropdownInput extends Component {

SearchDropdownInput.defaultProps = {
maxResults: 250,
items: [],
};
SearchDropdownInput.propTypes = {
items: PropTypes.arrayOf(
Expand All @@ -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
Expand Up @@ -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>
);
Expand Down
Loading