Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use prettier #18700

Closed
wants to merge 1 commit into from
Closed

Use prettier #18700

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 22, 2017

Fixes #18340

WIP -- Blocked on arijs/prettier-miscellaneous#29
Does not include actually applying prettier; most of the changes are due to the issue mentioned above.
You can test it out in a checkout by using gulp run prettier.
(We could use vanilla pretiter but then most of the changes would be moving every else branch up a line.)
Also includes a precommit hook to ensure we're actually running it.
We should also be sure to apply this after any huge PRs are in, such as #17269.

@ghost ghost requested a review from weswigham September 22, 2017 21:02
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

I don't particularly mind running a second style tool; but we should probably also run tslint with --fix if we're going to start down the path of automatic fixes.

});

gulp.task("default", "Runs 'local'", ["local"]);

gulp.task("watch", "Watches the src/ directory for changes and executes runtests-parallel.", [], () => {
gulp.watch("src/**/*.*", ["runtests-parallel"]);
});

function withFold(name: string, action: () => void): void {
Copy link
Member

Choose a reason for hiding this comment

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

Why not withFold<T>(name: string, action: () => T): () => T, making the body:

return () => {
   if (fold.isTravis()) {
        console.log(fold.start(name));
    }
    const result = action();
    if (fold.isTravis()) {
        console.log(fold.end(name));
    }
    return result;
}

Allowing you to simply wrap whatever gulp task function you please? Should reduce nesting a bit, too (and clean up the diff).
Technically the fold won't work correctly for stream or promise return values... but w/e, it doesn't already and the higher-order definition looks a bit nicer at the use-site.

@Jessidhia
Copy link

(Anecdote warning) In my experience tslint --fix was dangerous in that it could introduce invalid or unintended syntax, but the last time I encountered this problem was like a year ago so it's possible it is reliable now.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 8, 2017

this has gone stale. closing for now.

@mhegazy mhegazy closed this Nov 8, 2017
@ghost ghost deleted the prettier branch November 8, 2017 17:57
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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.

5 participants