Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fix #532, Fix #566, add tslint in ci, add tslint/format/test/karma in precommit of git #565

Merged
merged 11 commits into from
Jan 12, 2017

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Dec 27, 2016

Fix #532, Fix #566

  1. Add tslint in Travis CI
  2. Modify some source codes to pass tslint.
  3. Add following precommit check.
  • tslint
  • format
  • karma singlerun with auto close ws-server
  • test/node

in git pre-commit with ghooks.
So we can make sure everything is fine before we commit with git.

@JiaLiPassion JiaLiPassion changed the title Fix #532, add tslint in ci, add tslint/format/test/karma in precommit of git Fix #532, Fix #566, add tslint in ci, add tslint/format/test/karma in precommit of git Dec 27, 2016
@@ -22,6 +22,7 @@ before_script:
- ./scripts/sauce/sauce_connect_block.sh

script:
- node_modules/.bin/tslint -c tslint.json 'lib/**/*.ts' 'test/**/*.ts'
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this covered by format:enforce ?
if not what about creating a gulp task ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the review, you are right, there already has a task in gulp 'gulp lint', I will use that one.

Pre-commit hook will be executed and following check will
automatically run before commit.

- tslint
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm -1 on creating a commit hook.
Test should run in watch mode anyway ?

Copy link
Collaborator Author

@JiaLiPassion JiaLiPassion Dec 29, 2016

Choose a reason for hiding this comment

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

Yeah, test in monitor mode can make sure all jasmine test passed.
But with pre commit, the hook can check format and lint.

Sometimes I forget to do gulp format and gulp lint, and make a commit, the format error was found by Travis, I just want to let developer find the error before commit.

Are you suggesting when code changes, automatically run format and lint check?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you suggesting when code changes, automatically run format and lint check?

you can set up your IDE/editor for that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, I see, I have removed the hook

@@ -15,11 +15,19 @@
},
"scripts": {
"changelog": "gulp changelog",
"ci": "npm run lint && npm run format && npm run test:single && npm run test-node",
Copy link
Contributor

Choose a reason for hiding this comment

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

document those in the README ?

Copy link
Collaborator Author

@JiaLiPassion JiaLiPassion Dec 29, 2016

Choose a reason for hiding this comment

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

Yes, I will organize all those scripts and add them into DEVELOPER.md

@JiaLiPassion
Copy link
Collaborator Author

@vicb, I have updated the PR, and about pre-commit hook, I still keep it because I think we should check lint and format before commit. Please check whether it is a correct way or not. Or maybe we should just add lint and format in pre-commit hook and leave test alone?

@JiaLiPassion
Copy link
Collaborator Author

JiaLiPassion commented Dec 30, 2016

@vicb, I have removed the hook, please review, thank you.

@mhevery mhevery merged commit fb8d51c into angular:master Jan 12, 2017
@JiaLiPassion JiaLiPassion deleted the precommit branch January 12, 2017 01:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should add precommit hook to do tslint/format/karma/node test Run ts-lint as part of the CI
4 participants