Skip to content
This repository has been archived by the owner. It is now read-only.

add tslint to the Fastfile #658

Merged
merged 7 commits into from Apr 13, 2018

Conversation

Projects
None yet
4 participants
@nakhbari
Copy link
Contributor

nakhbari commented Apr 12, 2018

#657

Now we can add a project that runs tslint on the web typescript files!

Sample Output
/Library/Ruby/Gems/2.3.0/gems/fastlane-2.85.0/fastlane_core/lib/fastlane_core/ui/interface.rb:145:in `shell_error!': [!] Exit status of command 'cd ..;./node_modules/tslint/bin/tslint './web/**/*.ts'' was 2 instead of 0. (FastlaneCore::Interface::FastlaneShellError)

Warning: The 'deprecation' rule requires type information.
Warning: The 'no-use-before-declare' rule requires type information.

ERROR: web/app/dashboard/dashboard.component.ts[13, 57]: Missing semicolon
ERROR: web/app/models/project_summary.ts[1, 86]: Missing semicolon
@KrauseFx

This comment has been minimized.

Copy link
Member

KrauseFx commented Apr 12, 2018

Sorry for the noob question on this one, is TSLint like prettier?


lane :tslint do
desc "TSLint the whole project to ensure no style violations"
sh("cd ..;npm install")

This comment has been minimized.

@taquitos

taquitos Apr 12, 2018

Member

how do we guarantee that npm is installed?

This comment has been minimized.

@nakhbari

nakhbari Apr 12, 2018

Author Contributor

good point. I following bundle as an example, but i guess there's a higher assumption that it's already installed if you can run fastlane.

Any suggestions? I'll look into it in the meantime.

This comment has been minimized.

@nakhbari

nakhbari Apr 12, 2018

Author Contributor

@taquitos wouldn't the command fail, thus failing the fastlane execution? If so, that should be OK. doing a brew install npm might be overkill.

This comment has been minimized.

@taquitos

taquitos Apr 12, 2018

Member

Yeah, this is fine.
Let's add to the rake file whatever is the standard way of getting npm

This comment has been minimized.

@nakhbari

nakhbari Apr 13, 2018

Author Contributor

done

@nakhbari

This comment has been minimized.

Copy link
Contributor Author

nakhbari commented Apr 12, 2018

@KrauseFx I'm actually not too familiar with prettier. It seems like it's an automatic formatter? I have tslint configured in my VSCode to automatically format. I can provide instruction on this. Are there other functions prettier provides? I noticed there are articles on combining the two.

@KrauseFx

This comment has been minimized.

Copy link
Member

KrauseFx commented Apr 12, 2018

I don't know enough about JS, I just heard good things about prettier and how many teams make use of it. Let's stick with what you already set up 👍

@nakhbari

This comment has been minimized.

Copy link
Contributor Author

nakhbari commented Apr 12, 2018

@snatchev Can you help with adding this to git presubmit. Not really knowledgeable about that.

@taquitos

This comment has been minimized.

Copy link
Member

taquitos commented Apr 12, 2018

@snatchev snatchev self-assigned this Apr 13, 2018

@snatchev

This comment has been minimized.

Copy link
Member

snatchev commented Apr 13, 2018

@nakhbari added to the pre-commit hook in 03c3e32

One caveat: rubocop allows us to specify which ruby files to lint (in the changed_files array), and we only lint the ones that are about to be committed. I could not find a similar feature in tslint. Do you know if that is supported?

@snatchev
Copy link
Member

snatchev left a comment

LGTM, plus a small question about tslint

@nakhbari

This comment has been minimized.

Copy link
Contributor Author

nakhbari commented Apr 13, 2018

@snatchev I added a change to only do ng lint if there are modified .ts files.

you can select multiple files with tslint, but ng lint does some extra stuff and it's a project based check for typing purposes.

nakhbari added some commits Apr 13, 2018

@snatchev

This comment has been minimized.

Copy link
Member

snatchev commented Apr 13, 2018

@nakhbari 👍

@nakhbari nakhbari merged commit def085f into master Apr 13, 2018

4 checks passed

cla/google All necessary CLAs are signed
coverage/coveralls Coverage remained the same at 55.315%
Details
fastlane.ci: All rspecs pass All green
fastlane.ci: All rubocops pass All green

@nakhbari nakhbari deleted the tslint-fast branch Apr 13, 2018

@nakhbari nakhbari referenced this pull request Apr 13, 2018

Closed

Add TS Lint to Fastfile #657

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.