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

Set esSpecCompliant for trailing-comma rule in tslint:recommended #3899

Closed
wants to merge 2 commits into from
Closed

Set esSpecCompliant for trailing-comma rule in tslint:recommended #3899

wants to merge 2 commits into from

Conversation

chaucerbao
Copy link

PR checklist

Overview of change:

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

[rule-change] Set esSpecCompliant for trailing-comma rule in tslint:recommended

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @chaucerbao! 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.

@chaucerbao
Copy link
Author

I can't help but feel like the CI failure is not related to this update.

@david-cannady
Copy link
Contributor

Yeah same happened to my PR. The issue seems to exist in master. Though, none of the tests failed in my local run.

@vtambourine
Copy link

Is there any more blockers on this issue? Really want to see it in the nearest build!

@johnwiseheart
Copy link
Contributor

@chaucerbao can you confirm whether you have signed the CLA? This should be good to merge.

@chaucerbao
Copy link
Author

I apologize, I have not gone through the license agreement, not because I don't agree with it, I'm just not comfortable linking my account information to an external app as part of that process.

You (or anyone) are welcome to close this PR and recreate the update. There's no logic, just a preference to be compliant by default.

@johnwiseheart
Copy link
Contributor

Unfortunately we are unable to take your change. Hope that you feel comfortable to sign the CLA in the future.

@chaucerbao
Copy link
Author

No worries, I understand.

If you also agree that presets should be compliant by default, you can add it internally at Palantir. Or @vtambourine, if you're up for it you can make a PR.

@vtambourine
Copy link

@johnwiseheart now I'm confused. Proposed changes is a single line of logicless code, basically a configuration option, already described in TSLint docs that can hardly make any harm. Two comments ago you said this should be good to merge. What makes it unable to take your change now? What kind of bureaucratic nuance hidden in the CLA that prevents you to accept this change and force someone non-collaborator to sign this agreement?

It also worth to mention, that Palantir made impossible to read CLA before authorizing with personal GitHub account.

@johnwiseheart
Copy link
Contributor

Palantir requires that all contributors agree to the terms and conditions in the CLA prior to submitting any contributions. I said this PR was good to merge assuming that @chaucerbao had signed the CLA, as the cla-bot has been having some issues of recent and there has been some users who have indeed signed it but it has not updated the status. Because the CLA has not been signed, we cannot accept a contribution from @chaucerbao.

I agree that it should be possible to view the CLA before authorizing with your account, and will pass that on to the team.

@vtambourine
Copy link

vtambourine commented Jul 19, 2018 via email

@johnwiseheart
Copy link
Contributor

johnwiseheart commented Jul 19, 2018

Yes, it is possible for other contributors to make the same PR.

Yes, it is possible for me or any of the other maintainers to implement small and useful features. At the moment, most of our time is spent maintaining the project by reviewing PRs and responding to issues rather than developing features. I hope that one day we will have the time to work on new features ourselves too, but as we all have jobs separate to maintaining tslint, it is difficult to both develop new features as well as (attempt to) keep the number of PRs and issues low.

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