elm-community / html-extra

Additional functions for working with Html.
http://package.elm-lang.org/packages/elm-community/html-extra/latest
MIT License
31 stars 14 forks source link

Add an elm-review rule to warn of #26

Closed leojpod closed 4 years ago

leojpod commented 4 years ago

Hello!

I've been using elm-review and I wanted to make a rule to prevent using text "" all around my codebase.

Let me know what you think of it!

leojpod commented 4 years ago

Would it be ok to add a Githubaction workflow to run those elm-test tests?

prikhi commented 4 years ago

Hey! Thanks for the PR. I'm kinda leaning towards thinking this should be in a separate repository though - unless it's common to include elm-review rules in packages? I'd be happy to include a link to the repo in the documentation for the nothing value though.

leojpod commented 4 years ago

Hi @prikhi thanks for the feedback :)

Truth be told I don't know if it's common, it just sounded like a good way to feed 2 birds with one seed 😄 : keep 2 related things together and make more people aware of elm-review 😄 (and making a repository just for the rule sounded like a bit of a strech)

I'll do that though (making a repo). It can always be merged in later if more rules appear.

jfmengels commented 4 years ago

Hi all, elm-review author here :wave:

I personally think elm-review rules should be in separate repositories. A few reasons:

I agree that it would be nicer to have a package with several rules, rather than one with a single rule. @leojpod Maybe you can propose this rule https://github.com/jfmengels/elm-review-rule-ideas, have the rule in a repository under your name, (publish it or not if you want to) and see if other related rules are proposed in a way that would make sense to group them together.

prikhi commented 4 years ago

I personally think elm-review rules should be in separate repositories. A few reasons:

* The maintainers of the (this) package may not know how to nor want to maintain an `elm-review` rule (but if you want to, sure!)

* Adding `elm-review` adds a few dependencies (including `elm-test` and its load of dependencies!) that you probably don't want to have in the projects that use `elm-community/html-extra`.

@leojpod Maybe you can propose this rule https://github.com/jfmengels/elm-review-rule-ideas, have the rule in a repository under your name, (publish it or not if you want to) and see if other related rules are proposed in a way that would make sense to group them together.

Thanks for checking this out! I was thinking about the dependency load, but also I don't use elm-review at the moment & wouldn't be able to evaluate PRs or maintain the rules...

@leojpod, I'll leave this open for now so you can incorporate the review suggestions if you wish, but please close it once you have everything resolved. If you want to make a separate PR that mentions the rule in the nothing docs, I'll gladly merge that.

leojpod commented 4 years ago

Thanks for the feedback people :)

I think I'll create a separate repo for now (taking into account the comments) and make an issue in the review rule ideas.

Cheers!

leojpod commented 4 years ago

Thanks for the feed back, the package is published under https://package.elm-lang.org/packages/leojpod/review-no-empty-html-text/latest/