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

Address xo linting errors #2403

Closed
gr2m opened this issue Mar 29, 2022 · 4 comments · Fixed by #2408
Closed

Address xo linting errors #2403

gr2m opened this issue Mar 29, 2022 · 4 comments · Fixed by #2408

Comments

@gr2m
Copy link
Member

gr2m commented Mar 29, 2022

Follow up to #2402 (comment)

Current behavior

npm run lint

  test/definitions/branches.test.js:5:29
  ✖   5:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖   6:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖   7:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖   8:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖   9:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  10:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  11:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  13:30  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  14:30  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  15:30  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  16:30  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  17:30  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  40:28  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  41:28  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  42:28  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  44:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  45:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  46:29  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  69:25  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  71:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  72:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  73:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  74:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  75:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator
  ✖  76:26  Do not pass function directly to .filter(…).         unicorn/no-fn-reference-in-iterator

  lib/branches/index.js:29:54
  ✖  29:54  Array#reduce() is not allowed                        unicorn/no-reduce
  ✖  30:69  Do not pass function filter directly to .filter(…).  unicorn/no-fn-reference-in-iterator
  ✖  34:46  Array#reduce() is not allowed                        unicorn/no-reduce

  lib/plugins/index.js:14:34
  ✖  14:34  Array#reduce() is not allowed                        unicorn/no-reduce
  ✖  47:59  Array#reduce() is not allowed                        unicorn/no-reduce

  lib/definitions/branches.js:19:43
  ✖  19:43  Do not pass function branch directly to .filter(…).  unicorn/no-fn-reference-in-iterator
  ✖  19:73  Do not pass function branch directly to .filter(…).  unicorn/no-fn-reference-in-iterator

  lib/get-config.js:28:33
  ✖  28:33  Array#reduce() is not allowed                        unicorn/no-reduce
  ✖  35:12  Array#reduce() is not allowed                        unicorn/no-reduce

  lib/definitions/plugins.js:26:17
  ✖  26:17  Array#reduce() is not allowed                        unicorn/no-reduce

  lib/branches/expand.js:8:19
  ✖   8:19  Array#reduce() is not allowed                        unicorn/no-reduce

  cli.js:11:16
  ✖  11:16  Array#reduce() is not allowed                        unicorn/no-reduce

If someone could send a PR to address this it would be most appreciated. Both @travi and I won't be able to work on this anytime soon

@travi
Copy link
Member

travi commented Mar 30, 2022

i think the unicorn/no-fn-reference-in-iterator violations should be pretty simple to address.

i'm not sure i'm on board with enforcing the unicorn/no-reduce rule, though. the motivations for the rule are readability and performance. the performance concern is only really in cases where mutability is avoided within the loop. even if we are doing that in some of these cases, i would be surprised if it would be working with large enough lists to make a meaningful difference. as far as readability goes, i think that is a pretty subjective thing and should consider idiom familiarity of the maintainers. i don't mind reduce and would like to avoid reworking things that aren't truly needed at this point.

i dont know xo well, so it would be nice to understand if it is possible to disable the unicorn/no-reduce rule. assuming it is, is there a reason to keep that one enabled?

@gr2m
Copy link
Member Author

gr2m commented Mar 30, 2022

i'm not sure i'm on board with enforcing the unicorn/no-reduce rule, though

same, we can disable it.

@travi
Copy link
Member

travi commented Mar 30, 2022

ah, looks easy enough to disable here:

"rules": {
"unicorn/string-content": "off"
}

travi added a commit that referenced this issue Apr 2, 2022
the performance and readability of reduce is not a concern in this project since maintainers are
familiar with the idiom and are iterating over small lists. the filter rule is disabled selectively
since the filter being identified is not Array.filter

closes #2403
@gr2m gr2m closed this as completed in #2408 Apr 2, 2022
gr2m pushed a commit that referenced this issue Apr 2, 2022
the performance and readability of reduce is not a concern in this project since maintainers are
familiar with the idiom and are iterating over small lists. the filter rule is disabled selectively
since the filter being identified is not Array.filter

closes #2403
@github-actions
Copy link

github-actions bot commented Jun 9, 2022

🎉 This issue has been resolved in version 19.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

morey-tech pushed a commit to ratehub/semantic-release that referenced this issue Sep 12, 2022
…-release#2408)

the performance and readability of reduce is not a concern in this project since maintainers are
familiar with the idiom and are iterating over small lists. the filter rule is disabled selectively
since the filter being identified is not Array.filter

closes semantic-release#2403
adityahex27 pushed a commit to hextrust/semantic-release that referenced this issue Oct 31, 2022
…-release#2408)

the performance and readability of reduce is not a concern in this project since maintainers are
familiar with the idiom and are iterating over small lists. the filter rule is disabled selectively
since the filter being identified is not Array.filter

closes semantic-release#2403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@gr2m @travi and others