codeleague / codecoach Goto Github PK

View Code? Open in Web Editor NEW

8.0 9.0 13.0 213 KB

Automated code review tool

Home Page: https://github.com/codeleague/codecoach

JavaScript 4.09% TypeScript 95.68% Dockerfile 0.24%

automated-code-review code-review lint-review cli-app nodejs-app

codecoach's People

Contributors

Stargazers

Watchers

Forkers

saketgupte toucham goggags123 sorathank pooh-chan satishojhaa icmpecho jeeyo krishnamurthysvg vedmax aswathprabhu affankhushnod1994 yuhaibao39

codecoach's Issues

Remove Agents & Archive Git.ts

Able to detect CRLF and LF

In Config.ts and .env should remove APP_LOG_LINE_SPLITTER and the system should detect it automatically

Publish project to registry

so we can

$ yarn global add codecoach

and then

$ codecoach --pr=1234 --token=5678 ....

Move Message Template Function

some method createMessageWithEmoji or createComment should move out from Provider and have own class maybe Provider.utils

Automated CHANGELOG.md

Add step in publish-npm.yml flow to use release note to prepend CHANGELOG.md

Beta/Experimental release flow

Allow beta flow to be released from PRs (possibly automate ?)
Tag next / beta / experimental in npm
Restrict release flow to main branch

Open source preps

Remove secrets
Remove identities (from test cases, scripts, etc.)
Squash merge history (plus migrate master branch to main)
master branch to be removed
Remove old released packages (those containing secrets and identities)
Request GitHub to remove PR that contain identities and secrets
Make it public 🎉

Further steps

Publish to npm/yarn instead of GitHub packages
Refine README
Make CONTRIBUTING.md

Contribution appreciation in README

List down contributors with category (linter support, feature, bug fixes, documentation)
Add CONTRIBUTION doc to let contributor add themself to contributor section in README

Integration test

So we don't have to test branches on actual project

CodeCoach tries to comments on untouched line

CodeCoach tries to comment on the issue that occurs in the touched file but not touched line.

For example, File A was changed in line between 10-50. CodeCoach reports one issue (issueA) on line 15. But, there is another issue(issueB) from dotnet build on line 80.

CodeCoach, with its behavior, reports every issue in a touched file. So, it will try to report both issueA and issueB

But, GitHub does not allow us to comment on the untouched line (where issueB occurs). Thus, CodeCoach fails here.

From two pictures above, line: 857 is not touched in the pr. So, it can't be commented on.

https://teamcity.agodadev.io/viewLog.html?buildId=26164537&buildTypeId=AgodaFrontEnd_AgodaGateway_CodeAnalysisTest_CodeCoach&tab=buildLog&_focus=1514

https://github.agodadev.io/agoda-front-end/Agoda.Gateway/blob/a27d1681dff00db80037d0195c6cbb5bb29f621d/Tests/Agoda.Gateway.External.Tests/BookingCreation/BookingCreationRepositoryTests.cs#L865

Crash on MSBuild

log

[10:55:58]Collecting changes in 2 VCS roots (14s)
[10:59:34]The build is removed from the queue to be prepared for the start
[10:59:34]Starting the build on the agent "hk-unix-1630380965918"
[10:59:34]Updating tools for build
[10:59:34]Clearing temporary directory: /opt/buildagent/temp/buildTmp
[10:59:34]Publishing internal artifacts (3s)
[10:59:34]Checkout directory: /opt/buildagent/work/cb585e686fd79a0a
[10:59:34]Resolving artifact dependencies
[10:59:34]Step 1/2: Run (Command Line) (19s)
[10:59:34][Step 1/2] Swabra
[10:59:34][Step 1/2] Starting: /opt/buildagent/temp/agentTmp/custom_script4594896988933018394
[10:59:34][Step 1/2] in directory: /opt/buildagent/work/cb585e686fd79a0a
[10:59:34][Step 1/2] PR 420
[10:59:34][Step 1/2] MSBuild execution path C:/buildagent/work/1c22f4d3d9030336
[10:59:34][Step 1/2] ESLint execution path C:/buildagent/work/1c22f4d3d9030336
[10:59:34][Step 1/2] Unable to find image 'reg-hk.agodadev.io/library/node:14-slim' locally
[10:59:34][Step 1/2] 14-slim: Pulling from library/node
[10:59:34][Step 1/2] babf97a3f00a: Pulling fs layer
[10:59:34][Step 1/2] d81eb1809d95: Pulling fs layer
[10:59:34][Step 1/2] 1d14a5c3c182: Pulling fs layer
[10:59:34][Step 1/2] 21d157458a67: Pulling fs layer
[10:59:34][Step 1/2] b358ef6b56c5: Pulling fs layer
[10:59:34][Step 1/2] 21d157458a67: Waiting
[10:59:34][Step 1/2] b358ef6b56c5: Waiting
[10:59:34][Step 1/2] d81eb1809d95: Verifying Checksum
[10:59:34][Step 1/2] d81eb1809d95: Download complete
[10:59:35][Step 1/2] 21d157458a67: Verifying Checksum
[10:59:35][Step 1/2] 21d157458a67: Download complete
[10:59:35][Step 1/2] b358ef6b56c5: Download complete
[10:59:35][Step 1/2] babf97a3f00a: Download complete
[10:59:35][Step 1/2] 1d14a5c3c182: Verifying Checksum
[10:59:35][Step 1/2] 1d14a5c3c182: Download complete
[10:59:36][Step 1/2] babf97a3f00a: Pull complete
[10:59:36][Step 1/2] d81eb1809d95: Pull complete
[10:59:39][Step 1/2] 1d14a5c3c182: Pull complete
[10:59:39][Step 1/2] 21d157458a67: Pull complete
[10:59:39][Step 1/2] b358ef6b56c5: Pull complete
[10:59:39][Step 1/2] Digest: sha256:3f0e71eee1467ac6d0a16ea66da16e1e0092c56d7e06ebaf2695b5de175cd4d9
[10:59:39][Step 1/2] Status: Downloaded newer image for reg-hk.agodadev.io/library/node:14-slim
[10:59:53][Step 1/2] /usr/local/bin/codecoach -> /usr/local/lib/node_modules/codecoach/dist/app.js
[10:59:53][Step 1/2] + [email protected]
[10:59:53][Step 1/2] added 83 packages from 50 contributors in 13.012s
[10:59:54][Step 1/2] error: MSBuildParser Error: log structure doesn't match {"log":"C:\\buildagent\\work\\1c22f4d3d9030336\\src\\Agoda.WebSite.MVC.Api\\Agoda.WebSite.MVC.Api.csproj : warning NU1701: Package 'Microsoft.AspNet.WebApi.Client 5.2.3' was restored using '.NETFramework,Version=v4.6.1' instead of the project target framework '.NETStandard,Version=v2.0'. This package may not be fully compatible with your project."}
[10:59:54][Step 1/2] error: Unexpected error MSBuildParser Error: log structure doesn't match {"stack":"Error: MSBuildParser Error: log structure doesn't match\n    at MSBuildParser.toLog (/usr/local/lib/node_modules/codecoach/dist/Parser/MSBuildParser.js:25:19)\n    at /usr/local/lib/node_modules/codecoach/dist/Parser/MSBuildParser.js:17:32\n    at Array.map (<anonymous>)\n    at MSBuildParser.parse (/usr/local/lib/node_modules/codecoach/dist/Parser/MSBuildParser.js:17:14)\n    at App.<anonymous> (/usr/local/lib/node_modules/codecoach/dist/app.js:60:31)\n    at Generator.next (<anonymous>)\n    at fulfilled (/usr/local/lib/node_modules/codecoach/dist/app.js:6:58)"}
[10:59:54][Step 1/2] error: Unexpected error {"error":{}}
[10:59:54][Step 1/2] Process exited with code 1
[10:59:54][Step 1/2] Process exited with code 1 (Step: Run (Command Line))
[10:59:54][Step 1/2] Step Run (Command Line) failed
[10:59:54]Step 2/2: Post check clean up (Cleanup docker data in Linux agent)
[10:59:54]Publishing internal artifacts (3s)
[10:59:54]Publishing artifacts (3s)
[10:59:58]Build finished

Document system architecture

Document should answer these following questions

How it's work in detail ?
How it should be deployed ?

Refine README

diagram of (lint outputs -> codecoach -> source controls) #12
goal of auto commenting pr (non gh action tho)
how cwd is important to codecoach

The C# rules is write for parse a line like this,
Broken.cs(6,8): warning AG0030: Prevent use of dynamic [C:\Users\kthuswongsa\Documents\codecoach\sample\csharp\broken.csproj]
that take Broken.cs for source and take (6,8) as a line and lineOffset,
the problem is
CSC : error CS5001: Program does not contain a static 'Main' method suitable for an entry point [C:\Users\kthuswongsa\Documents\codecoach\sample\csharp\broken.csproj]
this error have no line and lineOffset so it have to write a new rules to parse its.

Don’t remove old comments

For visibility and possibility of data collection.

(Can be a configurable behavior)

Merge issues on same line

And put them in a single review comment

Accept config file as another option to inject configs

Idea

We have command codecoach --<config1>=<value1> --<config2>=<value2> to execute CodeCoach.
Would be nice if we also have codecoach --config=<configFilePath> to read from.

And config file could be formatted like

repo:
  url: https://github.com/codeleague/codecoach
  pr: 40
  token: 123qwe123qwe123qwe123
buildLogFiles:
- type: tslint
  file: /path/to/log.json
  cwd: /current/dir/of/lint/output
- type: msbuild
  file: /path/to/output/log
  cwd: /path/of/cwd
output: /path/to/codecoach/output.json

maybe ?

also read stdin with yaml/json format too ?
default config filename like .codecoach ? So users can just codecoach

E2E test

Steps:

Generate fake build log based on CodeCoach PR changes
link CodeCoach build on CI
Run CodeCoach against it's own PR

Should codecoach handle git pulling ?

Since all CI systems already pulling source before executing anything.
If answer is no then we can remove Git.ts and --noClone flag.
less code!

CodeCoach should do actual PR review

not only PR status check but post 'Approve', 'Comment' or 'Request Change' to the PR

should be configurable options

Git clone via Git.ts give output via stderr instead of stdout

The code of git clone through exec of child_process is

const process = promisify(exec);
const git = await process(command.cmd.join(' '), { cwd: command.cwd });```

==> git.stderr has all output instead of git.stdout

The errors are the following:
--------
Error: Error: Cloning into './repo'...
    at GithubProvider.<anonymous> (C:\Users\bpuangthamaw\Documents\GitHub\codecoach\src\Provider\Github.provider.ts:39:13)
    at Generator.throw (<anonymous>)
    at rejected (C:\Users\bpuangthamaw\Documents\GitHub\codecoach\src\Provider\Github.provider.ts:6:65)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Deprecated AGENT_*_LOG_SETTING

refer to this .env file

APP_LOG_FILE_PATH=dotnetbuild.json
APP_WARN_FILE_PATH=dotnetbuild.wrn
APP_ERR_FILE_PATH=dotnetbuild.err

and

AGENT_ERROR_LOG_PATH=dotnetbuild.err
AGENT_WARN_LOG_PATH=dotnetbuild.wrn
AGENT_LOG_PATH=dotnetbuild.json

it duplicated settings, so AGENT should override or inherit from APP

LogSeverity enum not account for all Severity possible from Roslyn

according to https://github.com/thomaslevesque/GenerateCSharpErrors/blob/master/CSharpErrorsAndWarnings.md
there are 5 severity level:
Hidden Info Warning Error Fatal

but in LogSeverity:
Info Warning Error

Configurable GitHub instance. (GithubProvider)

in GithubProvider L#38 in PR #5

resource location is hardcoded to 'https://github.com/`

We need this to be configurable. May be create new config called resourceBaseUrl ?

Support Plugins

This is a work-in-progress proposal to add plugins support to CodeCoach.

This proposal may include the followings:

add support for JavaScript config file (codecoach.config.js)
provide standard plugin interface
may involve in removing yargs which means command line options and existing json config file will require rework

Use cwd to get relative path

Instead of manually copying target code in ./tmp/repo in CI.
Make sure to run CodeCoach at source root folder and use cwd to get relative path.