trolit / patchron Goto Github PK

performs initial pull request code review 🕵️. Inteded to faster code reviews, not substitute human one

Home Page: https://github.com/trolit/patchron-test/pulls

License: ISC License

Dockerfile 0.18% JavaScript 99.53% Shell 0.30%

automated-code-review code-review probot-apps github-bot node-app probot pull-request-review review-bot code-review-bot pr-review

patchron's Introduction

🐶 Patchron

GitHub bot 🤖 that performs initial pull request code review. It is intended to faster further code reviews, not substitute human ones 😎

versioned rules
built with Probot
easy to configure and expand
includes tests (Jest) and type definitions (jsdoc)
wrapped with own context to improve logging and accessing Probot's context

Disclaimer

review is based upon patches (to not overload GitHub API) which contain limited number of information. Due to that, some comments might be unrelevant. Despite of that, it comes to clicking resolve button while at the same time reviewers don't have to focus on simple things.

1. Setup

with Node.js

Clone repository.
Install dependencies.
```
npm install
```
Run the bot.
```
npm start
```
Follow further instructions from terminal to finish setup.

with Docker

Pull image from GHCR or build your own.

docker pull ghcr.io/trolit/patchron:latest

docker build -t patchron .

Obtain APP_ID and PRIVATE_KEY.

Install app via marketplace https://github.com/apps/patchron and configure repository access. Afterwards visit app https://github.com/settings/installations, note down APP_ID and generate PRIVATE_KEY.

Create running container from image (APP_ID and PRIVATE_KEY are mandatory)

docker run -e APP_ID=<app-id> -e PRIVATE_KEY=<pem-value> patchron

more options:
-e SENDERS=<usernames-separated-by-comma>
-e MAX_COMMENTS_PER_REVIEW=<number>

with GitHub Actions

Use GitHub Token or PAT

You can use GitHub token that is generated automatically on event (comments will be associated with github-actions bot) or personal access token to associate review with e.g. your own bot account. In that case you only need to add following snippet to repository workflow and adjust it to your needs.

name: Perform first PR review (GITHUB TOKEN)

on:
    pull_request:
        types:
            - opened

jobs:
    reviewOpenedPull:
        runs-on: ubuntu-latest
        steps:
            - uses: actions/checkout@v3
            with:
                repository: 'trolit/Patchron'
                ref: 'master'

            - run: npm ci --only=production

            - run: npm start
            # options: https://github.com/trolit/Patchron#2-configuration
            env:
                GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # or secrets.PAT
                # when 'github' is assigned, attempts to read variables directly from workflow
                NODE_ENV: 'github'

Use app installation token

⚠️ Note: Snippet uses @navikt/github-app-token-generator to generate app installation token. You can provide different solution (or your own). At this moment probot/adapter-github-actions does not support to generate app installation token by app id and private key.

Install app via marketplace https://github.com/apps/patchron
Configure repository access (repository that you want to be reviewed should be accessible by app).
Generate PRIVATE_KEY
Add APP_ID and PRIVATE_KEY secrets to repository

Use following snippet to add workflow in your repository.

name: Perform first PR review (APP INSTALLATION TOKEN)

on:
    pull_request:
        types:
            - opened

jobs:
    reviewOpenedPull:
        runs-on: ubuntu-latest
        steps:
            - uses: navikt/github-app-token-generator@v1
            id: get-token
            with:
                private-key: ${{ secrets.PRIVATE_KEY }}
                app-id: ${{ secrets.APP_ID }}

            - uses: actions/checkout@v3
            with:
                repository: 'trolit/Patchron'
                ref: 'master'

            - run: npm ci --only=production

            - run: npm start
            # options: https://github.com/trolit/Patchron#2-configuration
            env:
                GITHUB_TOKEN: ${{ steps.get-token.outputs.token }}
                # when 'github' is assigned, attempts to read variables directly from workflow
                NODE_ENV: 'github'

Rules configuration examples 🧩

Default (rules categorised by file extension)
Node (commonjs) + Vue in single repository (rules split by relative path and file extension)

How to arrange own rules 🤔

Rules config file is expected to be expressed as .json with specific structure to unify app behaviour between all available ways of serving it. It should have pull array and files object. Pull rules are intended to verify pull request data (not changes in files) and that's why it has separated section. You can manage configuration in two ways:

via extension

{
    "pull": [],
    "files": {
        "js": [],
        "vue": [],
        "cs": []
    }
}

It's used in repository as default configuration here. When app fetches files from pull request event, it takes each filename and attempts to get related rules from files object by file extension. If e.g. bot receives src/helpers/doSomething.js it will attempt to get rules from rules.files['js'].

via relative paths

There might be a case where single repository is used to store more app parts (e.g. client and server) and you would like to separate client rules from server (because for example server is in commonjs type and client in module). To solve it, you can group rules by relative paths:

{
    "pull": [],
    "files": [
        "server/*": {
            "js": [
                { }
            ]
        },
        "client/*": {
            "js": [
                { }
            ],
            "vue": [
                { }
            ]
        }
    ]
}

⚠️ End relative paths with asterisks (e.g. server/*) if you want to match files that are stored under server/ regardless of the nesting level.

server/*
- server/doSomething.js (matched)
- server/helpers/chart/saveLegendItems.js (matched)
server
- server/doSomething.js (matched)
- server/helpers/chart/saveLegendItems.js (not matched)

2. Configuration

Property	Type (default)	Description
`NODE_ENV`	String ( )	specifies environment in which app is running. For GitHub actions use `github`, for testing purposes `test` and for self hosted app `production`. GitHub POST comments, summary, approve actions are limited to `github` and `production` environments.
`RULES_CONFIGURATION_PATH`	String (`src/config/rules`)	Path to rules configuration file stored in project. Used when `RULES_CONFIGURATION_URL` is not provided.
`RULES_CONFIGURATION_URL`	String ( )	When provided, attempts to fetch rules configuration from given URL. URL should point to `.json` file (example structure).
`IS_GET_FILES_REQUEST_PAGINATED`	boolean (`false`)	Controls files fetching strategy. Unpaginated response includes a maximum of 3000 files which is sufficient in 99.9999999999% of cases.
`DELAY_BETWEEN_COMMENT_REQUESTS_IN_SECONDS`	Number (`3`)	After pull request review is done, delays time between each comment POST request to not overload GitHub API. Creating content too quickly may result in secondary rate limiting.
`IS_OWNER_ASSIGNING_ENABLED`	boolean (`true`)	When true, PR owner will be automatically assigned on issueing pull request.
`IS_REVIEW_SUMMARY_ENABLED`	boolean (`false`)	When true, at the end of the PR review, app will post summary that contains various information e.g. total comments that were successfully posted.
`IS_STORING_LOGS_ENABLED`	boolean (`false`)	When true, logs are also stored physically in `.logs` directory. Log files are named in following format: `YYYY-MM-DD`.
`MAX_COMMENTS_PER_REVIEW`	Number (`25`)	Limits number of comments that can be posted in single review under single pull request.
`SENDERS`	String ( )	Allows to limit pull requests reviews to certain users. Pass string with usernames separated by comma e.g. `'test1, test2, test3'`
`APPROVE_PULL_ON_EMPTY_REVIEW_COMMENTS`	boolean (`true`)	When true, approves pull request on empty review comments.

3. Useful links

4. Name origin

Name simply comes from merging two words: Patch and Patron 🐶

patchron's People

Contributors

Stargazers

Watchers

patchron's Issues

!rereview command

Add option that restricted number of users (or PR owner) can call !rereview command to re-run bot review

Allow to configure review paths (e.g. when both frontend and backend in single repo)

Currently Probot reviews files by file extension, regardless of their location.

Assume that one repository holds backend and frontend

If backend is not of type module, but frontend is, user should be allowed to map review. E.g.

backend/* -> js
frontend/* -> vue

Events should start from on [Vue]

<blabla
   @test1="onTest1Blabla"
/>

Rename project

Rework app to abandon global variables

Implicit index import rule [JS]

.../blabla/index.js"

.../blabla"

No Length Reading At Every Loop Iteration Rule

Loops can become very slow if you don’t do them right. One of the most common mistake is to read the length attribute of an array at every iteration:

var names = ['George','Ringo','Paul','John'];
for(var i=0;i<names.length;i++){
  doSomeThingWith(names[i]);
}

This means that every time the loop runs, JavaScript needs to read the length of the array. You can avoid that by storing the length value in a different variable:

var names = ['George','Ringo','Paul','John'];
var all = names.length;
for(var i=0;i<all;i++){
  doSomeThingWith(names[i]);
}

Object Definition Always Const Rule

when declaring object, suggest to always use const

Single Line Block Rule

Implement rule that allows to set pattern for conditional statements that include only single line

if (...)  // do something...

if (...) 
   // do something...
   
if (...) { 
   // do something...
}

do
    // something
while ();

do {
    // something
}
while ();

do 
{
    // something
}
while ();

Misspelled Name Rule

Use e.g. words collection to suggest words that were potentially misspelled?

⚠️

Would require ability to store words that should be ignored (via local file or DB)
What are the odds for custom names?

MarkedCommentsRule for HTML

Extend MarkedComments Rule and implement rule for HTML

rework MarkedCommentsRule to accept comment blocks

Configure typedefs in project (optimize documentation)

No Object Property Reassignment

// invalid
return {
   test: test
}

// valid
return {
   test
}

Force Object Literal Rule

var cow = new Object();
cow.colour = 'brown';
cow.commonQuestion = 'What now?';
cow.moo = function(){
  console.log('moo');
}
cow.feet = 4;
cow.accordingToLarson = 'will take over the world';

against

var cow = {
  colour:'brown',
  commonQuestion:'What now?',
  moo:function(){
    console.log('moo);
  },
  feet:4,
  accordingToLarson:'will take over the world'
};

Load bot configuration by passing URL to file

env should include property

URL_TO_BOT_CONFIGURATION=

when provided, bot on init fetches config, parses it and uses as active one. If URL is not provided, default (current) config is used.

Simple Short If Rule

Case 1

from:

return this.item?.total ? format(this.item.total) : 0;

to:

return format(this.item?.total || 0);

Case 2

from:

var direction;
if(x > 100){
  direction = 1;
} else {
  direction = -1;
}

to:

var direction = (x > 100) ? 1 : -1;

Case 3

from:

if(v){
  var x = v;
} else {
  var x = 10;
}

to:

var x = v || 10;

Positioned Keywords Rule - second layer of positioning

Upgrade rule to allow to force position order between different variations of keyword.

For import it would allow to specify order of:

packages
components
helpers

ATM only second layer of ordering is planned.

Add "note" prop to each rule

Predefined Filename Rule

configure directories that would require specific file naming convention. For instance,

{
  path: 'backend/controllers',
  filename: /.*Controller.js/
}

Line Break Before Return Rule

No Shorthand Variables Rule

suggest minimum length for const/let/var variable names

// case1
const [ a, b ] = [ 1, 2 ];

// case1.1
const [ a, b, c, d, e, f ] = [ 
   1,
   2,
   3,
   4,
   ...
]

// case2
const { a, b } = { a: 1, b: 2 };

// case3
const a = 1;
const b = 2;

// case4
const a = 1,
      b = 2;

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Warning

These dependencies are deprecated:

Datasource	Name	Replacement PR?
npm	`pino-multi-stream`

Rate-Limited

These updates are currently rate-limited. Click on a checkbox below to force their creation now.

Update Probot (major) (@probot/adapter-github-actions, probot)
Update utilities to v3 (major) (cron, node-fetch)
Update dev dependencies (nock, nodemon, smee-client)
Update dev dependencies (major) (nodemon, smee-client)
Update actions/checkout action to v4
Update docker/login-action action to v3
🔐 Create all rate-limited PRs at once 🔐

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

Update dependency probot to v12.4.0
Update utilities (cron, dayjs, dotenv, http-status-codes, js-base64, module-alias)

Detected dependencies

dockerfile

Dockerfile

github-actions

.github/workflows/publish.yml

actions/checkout v3

docker/login-action v2

.github/workflows/test.yml

actions/checkout v3

npm

package.json

@probot/adapter-github-actions 3.1.3

cron 2.1.0

dayjs 1.11.7

dedent-js 1.0.1

dotenv 16.0.3

dotenv-parse-variables 2.0.0

http-status-codes 2.2.0

js-base64 3.7.3

lodash 4.17.21

module-alias 2.2.2

node-fetch ^2.6.7

pino-multi-stream 6.0.0

pino-tee 0.3.0

probot 12.2.8

nock 13.2.9

nodemon 2.0.20

smee-client 1.2.3

Check this box to trigger a request for Renovate to run again on this repository

Length Initialized Before Loop Rule

Loops can become very slow if you don’t do them right. One of the most common mistake is to read the length attribute of an array at every iteration:

var names = ['George','Ringo','Paul','John'];
for(var i=0;i<names.length;i++){
  doSomeThingWith(names[i]);
}

This means that every time the loop runs, JavaScript needs to read the length of the array. You can avoid that by storing the length value in a different variable:

var names = ['George','Ringo','Paul','John'];
var all = names.length;
for(var i=0;i<all;i++){
  doSomeThingWith(names[i]);
}

Array Shortcut Rule

var awesomeBands = new Array();
awesomeBands[0] = 'Bad Religion';
awesomeBands[1] = 'Dropkick Murphys';
awesomeBands[2] = 'Flogging Molly';
awesomeBands[3] = 'Red Hot Chili Peppers';
awesomeBands[4] = 'Pornophonique';

var awesomeBands = [
  'Bad Religion',
  'Dropkick Murphys',
  'Flogging Molly',
  'Red Hot Chili Peppers',
];

Logical Operator Style

Aborted due to insufficient data to determine "extended" mode within patches
e.g. if (something) to comment it and suggest if (something === true) instead.

Instead of Logical Operator Style only simple mode rule will be introduced.

but there is window for that in later version 🤔

No Raw Indexing Rule

try to implement rule that enforces to reference element from array via index only once.

var f = document.getElementById('mainform');
var inputs = f.getElementsByTagName('input');

// wrong
for(var i=0,j=inputs.length;i<j;i++){
  if(inputs[i].className === 'mandatory' &&
     inputs[i].value === ''){
    inputs[i].style.borderColor = '#f00';
    inputs[i].style.borderStyle = 'solid';
    inputs[i].style.borderWidth = '1px';
  }
}

// correct
for(var i=0,j=inputs.length;i<j;i++){
  const input = inputs[i];

  if(input.className === 'mandatory' &&
    input.value === ''){
    input.style.borderColor = '#f00';
    input.style.borderStyle = 'solid';
    input.style.borderWidth = '1px';
  }
}

⚠️
Patch limited information problem :(

Longhand

if (isValid === true) {
 // do something ...
}

if (isValid === false) {
 // do something ...
}

Shorthand

if (isValid) {
 // do something ...
}

if (!isValid) {
 // do something ...
}

PS: more details about complications in rule's documentation

Import Specific Methods From Packages Rule

Implement rule that recommends to import/require functions directly from given packages.

e.g. for uniq from lodash

[1] import _ from 'lodash';

[2] import { uniq } from 'lodash';

[3] import uniq from 'lodash/uniq';

first and second way of importing should be commented out

trolit / patchron Goto Github PK

patchron's Introduction

🐶 Patchron

1. Setup

Use GitHub Token or PAT

Use app installation token

via extension

via relative paths

2. Configuration

3. Useful links

4. Name origin

patchron's People

Contributors

Stargazers

Watchers

patchron's Issues

Case 1

Case 2

Case 3

Rate-Limited

Open

Detected dependencies

Longhand

Shorthand

Recommend Projects

Recommend Topics

Recommend Org