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

Integrate this to tslint-react #8

Closed
jukben opened this issue Feb 12, 2019 · 8 comments
Closed

Integrate this to tslint-react #8

jukben opened this issue Feb 12, 2019 · 8 comments

Comments

@jukben
Copy link

jukben commented Feb 12, 2019

Hey! 👋

Thank you for great work, what do you think about to contribute this to tslint-react? palantir/tslint-react#186

@jukben jukben changed the title Integrate this back to tslint-react Integrate this to tslint-react Feb 12, 2019
@Gelio
Copy link
Owner

Gelio commented Feb 13, 2019

Hey 👋 I am all for it 👍 Actually, I was planning on making a PR today 😄

I am unsure whether https://github.com/palantir/tslint-react will accept it, though, as it will be the first external dependency with a rule that will be added to that repository 😄 We will see, though

@Gelio
Copy link
Owner

Gelio commented Feb 13, 2019

I have just filed a PR: palantir/tslint-react#204 🎉

@karfau
Copy link

karfau commented Feb 27, 2019

Hey there,
I agree it would make sense, but since over there there doesn't seem to be not to much progress (as is also the case with other PRs there)
I think it would be nice to just add the compiled rules to a dist folder in the repo (or do that with a postinstall hook maybe?) and providing a tslint.json to point to that directory.
This way your awesome rules can already be used directly from your github repo.

Since I would love to integrate your rules into https://github.com/bettermarks/bm-tslint-rules without relying on tslint-react to land your PR, that would help lot.

Any questions or comments on that topic are welcome!

Best Christian

@Gelio
Copy link
Owner

Gelio commented Feb 28, 2019

@karfau I am not sure if I understood you correctly 🙂 This project can already be installed on its own without tslint-react. The instructions are in the README. After installing and adding the project name in extends in tslint.json and specifying the rule severity (also in tslint.json), the rule should automatically work and apply in your code.

The rules are compiled just before they are published to npm. Try running npm install tslint-react-hooks and inspect the directory in node_modules. You will see that it looks slightly different than this repository

Adding this project to tslint-react could only make it more popular and more frequently downloaded, nothing else 🙂

@karfau
Copy link

karfau commented Feb 28, 2019

Awesome!
Don't know why I didn't realise it when scanning the README, I didn't check npm ... 😛
Adding a badge to the readme might make it more obviious. Not sure what I looked at yesterday...

@Gelio
Copy link
Owner

Gelio commented Feb 28, 2019

Which badge do you have in mind 😄? The readme currently contains badges for the number of downloads and the latest version published to npm 😄

@karfau
Copy link

karfau commented Feb 28, 2019

I think it won't be long until we will make use of it then. bettermarks/bm-tslint-rules#82

@Gelio
Copy link
Owner

Gelio commented Mar 13, 2019

Unfortunately, tslint-react wanted me to copy the implementation of this rule to their repo instead of adding it as a dependency. I would rather keep it in this repository so I can develop it freely. Therefore I am closing this PR 🙁

@Gelio Gelio closed this as completed Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants