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

Issue #72 -- Add a Linter #83

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
33 changes: 33 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# https://editorconfig.org
# Editor Config for WebMemex
# Download an extension for editor config to allow your IDE
# to follow the rules of the project

# Used to indicate the file is in root of the project
root = true

# Styling applicable to all forms of files
[*]
indent_style = space
indent_size = 4
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true
end_of_line = lf
charset = utf-8

# Styling applicable to the file extension given
[*.js, *.jsx]
# This is due to the fact that github allows only 119 columns of space to view
max_line_length = 119
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really discuss this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is a widely observed standard in many open source projects

max_line_length = 119

This is done so that there is no need for horizontal scrolling while reading the code as github provides only 119 columns worth of space to read the code. But I can change if it seems too restricting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Treora makes the final decision, I wouldn't have this rule

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason here is actually 120, but minus 1 char for the diff view. Why 120 I don't get because it seems pretty much browser specific, and people just choose a round number, see:
screenshot of github editor in different browsers on OsX
(source Django developers discussion)
For more random numbers, see this stackoverflow thread.

I do agree setting line lengths to something higher than 80, but 119 and binding that to Github web view and some browser specifics and then rounding that number seems pretty ... arbitrary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand github diff works on this length, I'm not discussing the number, I'm dicussing having any number there.
If tomorrow (real life story) I need to put an svg encoded icon somewhere I do want it on one line and I don't care if you have to scroll.
I'm not really worried of a dev writing a very very long line, why would it do that? So I consider this rule more a possible issue than a safeguard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point noted, I shall remove this line immediately. Or would you like another value to takes its place?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intended to ask about it too; adjusting rules to Github's presentation seems somewhat arbitrary (or submissive ;) ) indeed.

I have myself tried to keep things at a soft limit of 80 characters, though I'd be fine with a larger number too. As said it is nice to be able to make longer lines if needed; I am not sure whether editors interpret such a rule as soft or hard.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have a linter it will probably stops you from committing if you have linter errors, so I would say it will be pretty hard :)


[*.json]
indent_size = 2

# Ignore all the styles due to uglify
[*.min.js]
indent_style = ignore
insert_final_newline = ignore

[Makefile]
indent_style = tab
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extension/
59 changes: 59 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{
"env": {
"browser": true,
"webextensions": true
},
"extends": [
"plugin:import/recommended",
"plugin:import/react",
"plugin:promise/recommended",
"plugin:react/recommended",
"standard",
"standard-react"
],
"settings": {
"import/resolver": {
"node": {
"extensions": [
".js",
".jsx"
],
"moduleDirectory": [
"node_modules",
"."
]
}
}
},
"plugins": [
"standard",
"import",
"promise",
"react"
],
"parserOptions": {
"sourceType": "module",
"ecmaFeatures": {
"jsx": true
}
},
"rules":{
"comma-dangle": ["error", "always-multiline"],
"indent": ["error", 4, { "SwitchCase": 1 }],
"react/jsx-indent": ["error", 4],
"react/jsx-indent-props": ["error", 4],
"no-multiple-empty-lines": ["error", { "max": 2, "maxEOF": 0 }],
"no-unused-vars": "warn",
"space-before-function-paren": "off",
"operator-linebreak": ["error", "before"],
"import/no-mutable-exports": "error",
"import/unambiguous": "off",
"promise/avoid-new": "off",
"react/jsx-filename-extension": "error",
"react/prefer-stateless-function": "warn",
"react/sort-comp": "warn",
"react/jsx-max-props-per-line": [1, { "when": "multiline" }],
"react/no-unused-prop-types": "warn",
"react/prop-types": "off"
}
}
20 changes: 10 additions & 10 deletions gulpfile.babel.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ const files = [
entries: ['./src/overview/main.jsx'],
output: 'overview.js',
destination: './extension/overview',
cssOutput: 'style.css'
cssOutput: 'style.css',
},
{
entries: ['./src/options/main.jsx'],
output: 'options.js',
destination: './extension/options',
cssOutput: 'style.css'
}
cssOutput: 'style.css',
},
]

const browserifySettings = {
Expand All @@ -44,23 +44,23 @@ const browserifySettings = {
}

function createBundle({entries, output, destination, cssOutput},
{watch=false, production=false}) {
{watch = false, production = false}) {
let b = watch
? watchify(browserify({...watchify.args, ...browserifySettings, entries}))
.on('update', bundle)
: browserify({...browserifySettings, entries})
b.transform(babelify)
b.transform(envify({
NODE_ENV: production ? 'production' : 'development'
NODE_ENV: production ? 'production' : 'development',
}), {global: true})

if(cssOutput) {
if (cssOutput) {
b.plugin(cssModulesify, {
global: true,
output: path.join(destination, cssOutput),
postcssBefore: [
cssnext
]
cssnext,
],
})
}

Expand All @@ -71,10 +71,10 @@ function createBundle({entries, output, destination, cssOutput},
function bundle() {
let startTime = new Date().getTime()
b.bundle()
.on('error', error=>console.error(error.message))
.on('error', error => console.error(error.message))
.pipe(source(output))
.pipe(buffer())
.pipe(production ? uglify({output: {ascii_only:true}}) : identity())
.pipe(production ? uglify({output: {ascii_only: true}}) : identity())
.pipe(gulp.dest(destination))
.on('end', () => {
let time = (new Date().getTime() - startTime) / 1000
Expand Down
11 changes: 10 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
"build": "gulp build",
"watch": "gulp watch",
"fx-run": "web-ext -s extension run",
"fx-build": "npm run build-prod && web-ext -s extension -a dist/firefox build"
"fx-build": "npm run build-prod && web-ext -s extension -a dist/firefox build",
"lint": "eslint --ext js,jsx .",
"lint-fix": "eslint --ext js,jsx --fix ."
},
"dependencies": {
"babel-runtime": "^6.23.0",
Expand Down Expand Up @@ -48,6 +50,13 @@
"babelify": "^7.3.0",
"browserify": "^13.1.1",
"css-modulesify": "^0.27.1",
"eslint": "^3.18.0",
"eslint-config-standard": "^7.1.0",
"eslint-config-standard-react": "^4.3.0",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-promise": "^3.5.0",
"eslint-plugin-react": "^6.10.3",
"eslint-plugin-standard": "^2.1.1",
"gulp": "^3.9.1",
"gulp-identity": "^0.1.0",
"gulp-uglify": "^2.0.1",
Expand Down
4 changes: 2 additions & 2 deletions src/activity-logger/background/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import maybeLogPageVisit from './log-page-visit'
// Listen for navigations to log them and analyse the pages.
browser.webNavigation.onCommitted.addListener(({url, tabId, frameId}) => {
// Ignore pages loaded in frames, it is usually noise.
if (frameId !== 0)
if (frameId !== 0) {
return
}

maybeLogPageVisit({url, tabId})

})
7 changes: 4 additions & 3 deletions src/activity-logger/background/log-page-visit.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { generateVisitDocId, isWorthRemembering } from '..'
import db from 'src/pouchdb'
import { reidentifyOrStorePage } from 'src/page-storage/store-page'
import { generateVisitDocId, isWorthRemembering } from '..'


// Store the visit in PouchDB.
Expand All @@ -19,10 +20,10 @@ export default async function maybeLogPageVisit({
tabId,
url,
}) {

// First check if we want to log this page (hence the 'maybe' in the name).
if (!isWorthRemembering({url}))
if (!isWorthRemembering({url})) {
return
}

// The time to put in documents.
const timestamp = new Date().getTime()
Expand Down
2 changes: 1 addition & 1 deletion src/activity-logger/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const convertAnyTimestampedDocId = docuri.route(':type/:timestamp/:nonce')
export const getTimestamp = doc =>
Number.parseInt(convertAnyTimestampedDocId(doc._id).timestamp)

export function generateVisitDocId({timestamp, nonce}={}) {
export function generateVisitDocId({timestamp, nonce} = {}) {
const date = timestamp ? new Date(timestamp) : new Date()
return convertVisitDocId({
timestamp: date.getTime(),
Expand Down
2 changes: 1 addition & 1 deletion src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ browser.browserAction.onClicked.addListener(() => {
})

browser.commands.onCommand.addListener(command => {
if (command === "openOverview") {
if (command === 'openOverview') {
openOverview()
}
})
11 changes: 5 additions & 6 deletions src/browser-history/background/import-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { generatePageDocId } from 'src/page-storage'
async function getHistoryItems({
startTime = 0,
endTime,
}={}) {
} = {}) {
const historyItems = await browser.history.search({
text: '',
maxResults: 9999999,
Expand Down Expand Up @@ -88,8 +88,7 @@ function convertHistoryToPagesAndVisits({
const referringVisitDocId = referringVisitDoc._id
// Add a reference to the visit document.
visitDoc.referringVisit = {_id: referringVisitDocId}
}
else {
} else {
// Apparently the referring visit is not present in the history.
// We can just pretend that there was no referrer.
}
Expand All @@ -98,15 +97,15 @@ function convertHistoryToPagesAndVisits({
// Return the new docs.
return {
pageDocs,
visitDocs: Object.values(visitDocs) // we can forget the old ids now
visitDocs: Object.values(visitDocs), // we can forget the old ids now
}
}

// Pulls the full browser history into the database.
export default async function importHistory({
startTime,
endTime,
}={}) {
} = {}) {
// Get the full history: both the historyItems and visitItems.
console.time('import history')
const historyItems = await getHistoryItems({startTime, endTime})
Expand Down Expand Up @@ -148,6 +147,6 @@ export async function getOldestVisitTimestamp() {
export async function getHistoryStats() {
const historyItems = await getHistoryItems()
return {
quantity: historyItems.length
quantity: historyItems.length,
}
}
17 changes: 17 additions & 0 deletions src/dev/redux-devtools-component.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react'

import { createDevTools } from 'redux-devtools'
import LogMonitor from 'redux-devtools-log-monitor'
import DockMonitor from 'redux-devtools-dock-monitor'

const DevTools = createDevTools(
<DockMonitor
toggleVisibilityKey='ctrl-shift-L'
changePositionKey='ctrl-alt-shift-L'
defaultIsVisible={false}
>
<LogMonitor />
</DockMonitor>
)

export default DevTools
25 changes: 0 additions & 25 deletions src/dev/redux-devtools.jsx

This file was deleted.

Loading