Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[new-rule] add whitespace rule for open braces #4068

Merged
merged 1 commit into from
Aug 23, 2018

Conversation

erikseulean
Copy link
Contributor

@erikseulean erikseulean commented Jul 25, 2018

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Implements a whitespace option check-postbrace that is checking for a space, new line or tab after an open curly brace.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @erikseulean! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@erikseulean
Copy link
Contributor Author

Where can I read the CLA document? I cannot find it.

@jkillian
Copy link
Contributor

Hi @erikseulean, please see https://cla.palantir.com/, thanks!

@erikseulean
Copy link
Contributor Author

@jkillian I cannot find any terms there, it's only an authorization link for individual contributors but nothing rearding the content of the CLA.

@jkillian
Copy link
Contributor

Hi @erikseulean, turns out you have to authorize Github first before you can see the document. Obviously this isn't ideal, so I've pasted some screenshots of the document below so you can see it ahead of time:

image
image
image

@erikseulean
Copy link
Contributor Author

erikseulean commented Jul 26, 2018

Hi @jkillian thanks for this. I signed the CLA.

@erikseulean erikseulean changed the title add whitespace rule for open braces [new-rule] add whitespace rule for open braces Aug 6, 2018
@erikseulean
Copy link
Contributor Author

ping @jkillian Anybody up to look over this?

@erikseulean
Copy link
Contributor Author

@johnwiseheart do you think this brings some value to the existing codebase ? I'm happy to have this closed in case you don't consider it valuable to remove the clutter of long standing PRs.

@johnwiseheart
Copy link
Contributor

I think this added feature is worth having - does your addition also work with --fix? If so, it would be good to write a test for that.

@erikseulean
Copy link
Contributor Author

Hmm, not sure I understand exactly what test you want me to add. Could you please point me to a similar example ? I have already added a test, and a test fix, is it something else you asked me about or you missed those ?

@johnwiseheart
Copy link
Contributor

My mistake - I missed your fix test when reviewing. Thanks for your contribution!

@Jameskmonger
Copy link

When will this be published @johnwiseheart?

@johnwiseheart
Copy link
Contributor

johnwiseheart commented Dec 10, 2018 via email

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