Skip to content
This repository has been archived by the owner on Sep 2, 2022. It is now read-only.

build: fix bug, add lint in check #501

Merged
merged 1 commit into from
Nov 17, 2018
Merged

build: fix bug, add lint in check #501

merged 1 commit into from
Nov 17, 2018

Conversation

jnizet
Copy link
Member

@jnizet jnizet commented Nov 17, 2018

and fix lint errors

@codecov-io
Copy link

Codecov Report

Merging #501 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #501   +/-   ##
=========================================
  Coverage     86.03%   86.03%           
  Complexity      704      704           
=========================================
  Files           266      266           
  Lines          3474     3474           
  Branches        376      376           
=========================================
  Hits           2989     2989           
  Misses          279      279           
  Partials        206      206
Impacted Files Coverage Δ Complexity Δ
frontend/src/app/page-title.directive.ts 100% <100%> (ø) 0 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 180ae44...e9aee6a. Read the comment docs.

@jnizet jnizet merged commit e9aee6a into master Nov 17, 2018
@jnizet jnizet deleted the build/fixes branch November 17, 2018 00:16
delete(fileTree("${projectDir}/node_modules/moment/locale") {
include("*.js")
})
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what was the issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the classical newbie mistake. The code inside the closure configures the task, and is always run, whatever task you execute. I only want to delete when the task is executed, and the deletion must thus be inside a doLast block.

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, I introduced a bug. This task is of type Delete, and the delete method here configures what needs to be deleted. I missed the fact that is a was a Delete task, and thought I was talking project.delete(), which does delete immediately. So the code was correct from the start.

@@ -9,7 +9,7 @@
"test": "ng test --code-coverage --no-progress --no-watch",
"bundlesize": "bundlesize",
"codecov": "codecov",
"lint": "ng lint",
"lint": "ng lint > tslint-result.txt",
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why we output to a file? It removes the logs in the console right? Couldn't that be a reporter?

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK, there is no other way to have it generate a file (see angular/angular-cli#4959). I want a file to be generated otherwise gradle can't mark the task up-to-date. If it has been run before and nothing has changed.

Copy link
Member

Choose a reason for hiding this comment

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

I get it. I now remember that I use tslint directly in ng2ninja to output to a file https://github.com/Ninja-Squad/ng2ninja/blob/master/src/tslint.ts#L9

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants