Skip to content

Commit

Permalink
Document contribution to the code along with coding standards (#321)
Browse files Browse the repository at this point in the history
Fixes #320 
* Updated gulp.js to incrementally run the linter in watch mode
* Updated CONTRIBUTING.md with information on creating issues and contributing to code
* Added coding guidelines
  • Loading branch information
DonJayamanne authored Dec 1, 2017
1 parent 7ab96ed commit 5430b5e
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 45 deletions.
9 changes: 5 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ addons:
- g++-4.9-multilib
- libgtk2.0-0
- libx11-dev
- libxkbfile-dev
- libxkbfile-dev
- libsecret-1-dev
- python-dev
matrix:
Expand Down Expand Up @@ -41,8 +41,9 @@ before_install: |
git submodule update --init --recursive
git clone https://github.com/creationix/nvm.git ./.nvm
source ./.nvm/nvm.sh
nvm install 7.2.1
nvm use 7.2.1
nvm install 8.9.1
nvm use 8.9.1
npm i -g npm@5.5.1
npm config set python `which python`
if [ "$TRAVIS_OS_NAME" == "osx" ]; then
pyenv install $PYTHON
Expand All @@ -53,6 +54,6 @@ install:
- pip install --upgrade -r requirements.txt
- npm install
- npm run vscode:prepublish

script:
- npm test --silent
9 changes: 5 additions & 4 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"outFiles": [
"${workspaceFolder}/out/**/*.js"
],
"preLaunchTask": "compile"
"preLaunchTask": "Compile"
},
{
"name": "Launch Extension as debugServer", // https://code.visualstudio.com/docs/extensions/example-debuggers
Expand All @@ -30,7 +30,8 @@
"outFiles": [
"${workspaceFolder}/out/client/**/*.js"
],
"cwd": "${workspaceFolder}"
"cwd": "${workspaceFolder}",
"preLaunchTask": "Compile"
},
{
"name": "Launch Tests",
Expand All @@ -47,7 +48,7 @@
"outFiles": [
"${workspaceFolder}/out/**/*.js"
],
"preLaunchTask": "compile"
"preLaunchTask": "Compile"
},
{
"name": "Launch Multiroot Tests",
Expand All @@ -64,7 +65,7 @@
"outFiles": [
"${workspaceFolder}/out/**/*.js"
],
"preLaunchTask": "compile"
"preLaunchTask": "Compile"
}
],
"compounds": [
Expand Down
4 changes: 2 additions & 2 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"script": "compile",
"isBackground": true,
"problemMatcher": [
"$tsc",
"$tsc-watch",
{
"base": "$tslint5",
"fileLocation": "relative"
Expand All @@ -36,7 +36,7 @@
"panel": "shared"
},
"problemMatcher": [
"$tsc",
"$tsc-watch",
{
"base": "$tslint5",
"fileLocation": "relative"
Expand Down
46 changes: 46 additions & 0 deletions CODING_STANDARDS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
## Coding guidelines for TypeScript
* The following standards are inspired from [Coding guidelines for TypeScript](https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines).

### Names
* Use PascalCase for type names.
* Use "I" as a prefix for interface names only when an interface is implemented by a class.
* Use PascalCase for enum values.
* Use camelCase for function names.
* Use camelCase for property names and local variables.
* Do not use "_" as a prefix for private properties (unless used as backing properties).
* Use whole words in names when possible.

### Types

* Do not export types/functions unless you need to share it across multiple components.
* Do not introduce new types/values to the global namespace.
* Shared types should be defined in 'types.ts'.
Within a file, type definitions should come first.

### null and undefined

Use undefined. Do not use null.

### Comments

Use JSDoc style comments for functions, interfaces, enums, and classes.

### Strings

Use single quotes for strings.

### Style

* Use arrow functions over anonymous function expressions.
* Always surround loop and conditional bodies with curly braces. Statements on the same line are allowed to omit braces.
* Open curly braces always go on the same line as whatever necessitates them.
* Parenthesized constructs should have no surrounding whitespace.
* A single space follows commas, colons, and semicolons in those constructs. For example:
* `for (var i = 0, n = str.length; i < 10; i++) { }`
* `if (x < 10) { }`
* `function f(x: number, y: string): void { }`

* `else` goes on a the same line from the closing curly brace.
* Use 4 spaces per indentation.


49 changes: 34 additions & 15 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,45 +1,64 @@
## Contribution
* Please feel free to fork and submit pull requests
* Feature requests can be added [here](https://github.com/DonJayamanne/pythonVSCode/issues/183)

## Prerequisites
1. Node.js
### Prerequisites

1. Node.js (>= 8.9.1, < 9.0.0)
2. Python 2.7 or later (required only for testing the extension and running unit tests)
3. Windows, OS X or Linux
4. Visual Studio Code
5. Following VS Code extensions:
* [TSLint](https://marketplace.visualstudio.com/items?itemName=eg2.tslint)
* [EditorConfig for VS Code](https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig)

### Setup

## Setup
```
git clone https://github.com/DonJayamanne/pythonVSCode
cd pythonVSCode
git clone https://github.com/microsoft/vscode-python
cd vscode-python
npm install
```
## Development workflow

### Incremental Build
Run the build Task from the [Command Palette](https://code.visualstudio.com/docs/editor/tasks) (short cut CTRL+SHIFT+B or ⇧⌘B)

Run the `Compile` and `Hygiene` build Tasks from the [Command Palette](https://code.visualstudio.com/docs/editor/tasks) (short cut `CTRL+SHIFT+B` or `⇧⌘B`)

### Errors and Warnings
TypeScript errors and warnings will be displayed in VS Code in the Problems Panel (CTRL+SHIFT+M or ⇧⌘M)

TypeScript errors and warnings will be displayed in VS Code in the following areas:
* Problems Panel (`CTRL+SHIFT+M` or `⇧⌘M`)
* Terminal running the `Compile` task
* Terminal running the `Hygiene` task

### Validate your changes

To test the changes you launch a development version of VS Code on the workspace vscode, which you are currently editing.
Use the "Launch Extension" launch option.
Use the `Launch Extension` launch option.

### Unit Tests
Run the Unit Tests via the "Launch Test" launch option.
Currently unit tests only run on [Travis](https://travis-ci.org/DonJayamanne/pythonVSCode)

Run the Unit Tests via the `Launch Test` and `Launch Multiroot Tests` launch option.
Currently unit tests only run on [Travis](https://travis-ci.org/Microsoft/vscode-python)

_Requirements_
1. Ensure you have disabled breaking into 'Uncaught Exceptions' when running the Unit Tests
2. For the linters and formatters tests to pass successfully, you will need to have those corresponding Python libraries installed locally

## Debugging the extension
### Standard Debugging

Clone the repo into any directory and start debugging.
From there use the "Launch Extension" launch option.
From there use the `Launch Extension` launch option.

### Debugging the Python Extension Debugger

The easiest way to debug the Python Debugger (in our opinion) is to clone this git repo directory into [your](https://code.visualstudio.com/docs/extensions/install-extension#_your-extensions-folder) extensions directory.
From there use the ```Launch Extension as debugserver``` launch option.
From there use the ```Extension + Debugger``` launch option.

### Coding Standards

Information on our coding standards can be found [here](https://github.com/Microsoft/vscode-python/blob/master/CODING_STANDARDS.md).
We have a pre-commit hook to ensure the code committed will adhere to the above coding standards.


## Development process

Expand Down
89 changes: 69 additions & 20 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const colors = require('colors/safe');
* named according to the checks performed on them. Each subset contains
* the following one, as described in mathematical notation:
*
* all ⊃ eol ⊇ indentation ⊃ copyright ⊃ typescript
* all ⊃ eol ⊇ indentation ⊃ typescript
*/

const all = [
Expand Down Expand Up @@ -115,12 +115,12 @@ const hygiene = (some, options) => {
.toString('utf8')
.split(/\r\n|\r|\n/)
.forEach((line, i) => {
if (/^\s*$/.test(line)) {
if (/^\s*$/.test(line) || /^\S+.*$/.test(line)) {
// Empty or whitespace lines are OK.
} else if (/^(\s\s\s\s)+.*/.test(line)) {
// Good indent.
} else if (/^[\t]+.*/.test(line)) {
console.error(file.relative + '(' + (i + 1) + ',1): Bad whitespace indentation');
console.error(file.relative + '(' + (i + 1) + ',1): Bad whitespace indentation (use 4 spaces instead of tabs or other)');
errorCount++;
}
});
Expand All @@ -137,7 +137,7 @@ const hygiene = (some, options) => {
tsfmt: true
}).then(result => {
if (result.error) {
console.error(result.message);
console.error(result.message.trim());
errorCount++;
}
cb(null, file);
Expand All @@ -147,14 +147,14 @@ const hygiene = (some, options) => {
});
});

const program = require('tslint').Linter.createProgram("./tsconfig.json");
const linter = new tslint.Linter(options, program);
const tsl = es.through(function (file) {
const configuration = tslint.Configuration.findConfiguration(null, '.');
const options = {
formatter: 'json'
};
const contents = file.contents.toString('utf8');
const program = require('tslint').Linter.createProgram("./tsconfig.json");
const linter = new tslint.Linter(options, program);
linter.lint(file.relative, contents, configuration.results);
const result = linter.getResult();
if (result.failureCount > 0 || result.errorCount > 0) {
Expand Down Expand Up @@ -206,22 +206,16 @@ const hygiene = (some, options) => {
.pipe(filter(f => !f.stat.isDirectory()))
.pipe(filter(eolFilter))
.pipe(options.skipEOL ? es.through() : eol)
.pipe(filter(indentationFilter));

if (!options.skipIndentationCheck) {
result = result
.pipe(indentation);
}
.pipe(filter(indentationFilter))
.pipe(indentation);

// Type script checks.
let typescript = result
.pipe(filter(tslintFilter));
.pipe(filter(tslintFilter))
.pipe(formatting);

if (!options.skipFormatCheck) {
typescript = typescript
.pipe(formatting);
}
typescript = typescript.pipe(tsl)
typescript = typescript
.pipe(tsl)
.pipe(tscFilesTracker)
.pipe(tsc());

Expand All @@ -244,16 +238,32 @@ gulp.task('hygiene-staged', () => run({ mode: 'changes' }));
gulp.task('hygiene-watch', ['hygiene-staged', 'hygiene-watch-runner']);

gulp.task('hygiene-watch-runner', function () {
/**
* @type {Deferred}
*/
let runPromise;

return watch(all, { events: ['add', 'change'] }, function (event) {
// Damn bounce does not work, do our own checks.
const start = new Date();
if (runPromise && !runPromise.completed) {
console.log(`[${start.toLocaleTimeString()}] Already running`);
return;
}
console.log(`[${start.toLocaleTimeString()}] Starting '${colors.cyan('hygiene-watch-runner')}'...`);

runPromise = new Deferred();
// Skip indentation and formatting checks to speed up linting.
return run({ mode: 'watch', skipFormatCheck: true, skipIndentationCheck: true })
run({ mode: 'watch', skipFormatCheck: true, skipIndentationCheck: true })
.then(() => {
const end = new Date();
const time = (end.getTime() - start.getTime()) / 1000;
console.log(`[${end.toLocaleTimeString()}] Finished '${colors.cyan('hygiene-watch-runner')}' after ${time} seconds`);
});
runPromise.resolve();
})
.catch(runPromise.reject.bind);

return runPromise.promise;
});
});

Expand Down Expand Up @@ -402,7 +412,46 @@ function getModifiedFiles() {
});
});
}

// this allows us to run hygiene as a git pre-commit hook.
if (require.main === module) {
run({ exitOnError: true, mode: 'staged' });
}

class Deferred {
constructor(scope) {
this.scope = scope;
this._resolved = false;
this._rejected = false;

this._promise = new Promise((resolve, reject) => {
this._resolve = resolve;
this._reject = reject;
});
}
resolve(value) {
this._resolve.apply(this.scope ? this.scope : this, arguments);
this._resolved = true;
}
/**
* Rejects the promise
* @param {any} reason
* @memberof Deferred
*/
reject(reason) {
this._reject.apply(this.scope ? this.scope : this, arguments);
this._rejected = true;
}
get promise() {
return this._promise;
}
get resolved() {
return this._resolved === true;
}
get rejected() {
return this._rejected === true;
}
get completed() {
return this._rejected || this._resolved;
}
}

0 comments on commit 5430b5e

Please sign in to comment.